From 51171b84bf0a8c30b46170ee7a040bc6d655f54b Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Tue, 10 Sep 2019 22:24:49 +0200
Subject: [PATCH 01/45] main.metronome -> main.xmpp

---
 data/other/ldap_scheme.yml                  |  4 ++--
 data/templates/metronome/domain.tpl.cfg.lua |  2 +-
 src/yunohost/backup.py                      |  4 ++--
 src/yunohost/tests/test_permission.py       | 14 +++++++-------
 4 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/data/other/ldap_scheme.yml b/data/other/ldap_scheme.yml
index 11504bbe8..d013149af 100644
--- a/data/other/ldap_scheme.yml
+++ b/data/other/ldap_scheme.yml
@@ -67,8 +67,8 @@ depends_children:
             - permissionYnh
         groupPermission:
             - "cn=all_users,ou=groups,dc=yunohost,dc=org"
-    cn=main.metronome,ou=permission:
-        cn: main.metronome
+    cn=main.xmpp,ou=permission:
+        cn: main.xmpp
         gidNumber: "5002"
         objectClass:
             - posixGroup
diff --git a/data/templates/metronome/domain.tpl.cfg.lua b/data/templates/metronome/domain.tpl.cfg.lua
index 2ee9cfaae..d523365db 100644
--- a/data/templates/metronome/domain.tpl.cfg.lua
+++ b/data/templates/metronome/domain.tpl.cfg.lua
@@ -8,7 +8,7 @@ VirtualHost "{{ domain }}"
      hostname      = "localhost",
      user = {
        basedn        = "ou=users,dc=yunohost,dc=org",
-       filter        = "(&(objectClass=posixAccount)(mail=*@{{ domain }})(permission=cn=main.metronome,ou=permission,dc=yunohost,dc=org))",
+       filter        = "(&(objectClass=posixAccount)(mail=*@{{ domain }})(permission=cn=main.xmpp,ou=permission,dc=yunohost,dc=org))",
        usernamefield = "mail",
        namefield     = "cn",
        },
diff --git a/src/yunohost/backup.py b/src/yunohost/backup.py
index bd5d5750d..55b6678b8 100644
--- a/src/yunohost/backup.py
+++ b/src/yunohost/backup.py
@@ -1191,7 +1191,7 @@ class RestoreManager():
         old_apps_permission = []
         try:
             old_apps_permission = ldap.search('ou=permission,dc=yunohost,dc=org',
-                                              '(&(objectClass=permissionYnh)(!(cn=main.mail))(!(cn=main.metronome))(!(cn=main.sftp)))',
+                                              '(&(objectClass=permissionYnh)(!(cn=main.mail))(!(cn=main.xmpp))(!(cn=main.sftp)))',
                                               ['cn', 'objectClass', 'groupPermission', 'URL', 'gidNumber'])
         except:
             logger.info(m18n.n('apps_permission_not_found'))
@@ -1247,7 +1247,7 @@ class RestoreManager():
 
         # Remove all permission for all app which sill in the LDAP
         for per in ldap.search('ou=permission,dc=yunohost,dc=org',
-                               '(&(objectClass=permissionYnh)(!(cn=main.mail))(!(cn=main.metronome))(!(cn=main.sftp)))',
+                               '(&(objectClass=permissionYnh)(!(cn=main.mail))(!(cn=main.xmpp))(!(cn=main.sftp)))',
                                ['cn']):
             if not ldap.remove('cn=%s,ou=permission' % per['cn'][0]):
                 raise YunohostError('permission_deletion_failed',
diff --git a/src/yunohost/tests/test_permission.py b/src/yunohost/tests/test_permission.py
index d309a8211..3b9815f63 100644
--- a/src/yunohost/tests/test_permission.py
+++ b/src/yunohost/tests/test_permission.py
@@ -135,7 +135,7 @@ def check_LDAP_db_integrity():
 def check_permission_for_apps():
     # We check that the for each installed apps we have at last the "main" permission
     # and we don't have any permission linked to no apps. The only exception who is not liked to an app
-    # is mail, metronome, and sftp
+    # is mail, xmpp, and sftp
 
     from yunohost.utils.ldap import _get_ldap_interface
     ldap = _get_ldap_interface()
@@ -146,7 +146,7 @@ def check_permission_for_apps():
     installed_apps = {app['id'] for app in app_list(installed=True)['apps']}
     permission_list_set = {permission['cn'][0].split(".")[1] for permission in permission_search}
 
-    extra_service_permission = set(['mail', 'metronome'])
+    extra_service_permission = set(['mail', 'xmpp'])
     if 'sftp' in permission_list_set:
         extra_service_permission.add('sftp')
     assert installed_apps == permission_list_set - extra_service_permission
@@ -164,8 +164,8 @@ def test_list_permission():
     assert "main" in res['blog']
     assert "mail" in res
     assert "main" in res['mail']
-    assert "metronome" in res
-    assert "main" in res['metronome']
+    assert "xmpp" in res
+    assert "main" in res['xmpp']
     assert ["all_users"] == res['wiki']['main']['allowed_groups']
     assert ["alice"] == res['blog']['main']['allowed_groups']
     assert set(["alice", "bob"]) == set(res['wiki']['main']['allowed_users'])
@@ -220,9 +220,9 @@ def test_remove_bad_permission():
     assert "blog" in res
     assert "main" in res['blog']
     assert "mail" in res
-    assert "main" in res ['mail']
-    assert "metronome" in res
-    assert "main" in res['metronome']
+    assert "main" in res['mail']
+    assert "xmpp" in res
+    assert "main" in res['xmpp']
 
 def test_remove_main_permission():
     with pytest.raises(YunohostError):

From 0f688caccd33f425059081d0c283afce075e328b Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Tue, 10 Sep 2019 22:55:37 +0200
Subject: [PATCH 02/45] Swap 'main' in permission namespace

---
 data/other/ldap_scheme.yml                    | 8 ++++----
 data/templates/dovecot/dovecot-ldap.conf      | 4 ++--
 data/templates/metronome/domain.tpl.cfg.lua   | 2 +-
 data/templates/postfix/plain/ldap-accounts.cf | 2 +-
 data/templates/postfix/plain/ldap-aliases.cf  | 2 +-
 src/yunohost/app.py                           | 2 +-
 src/yunohost/backup.py                        | 2 +-
 7 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/data/other/ldap_scheme.yml b/data/other/ldap_scheme.yml
index d013149af..caa8fffb2 100644
--- a/data/other/ldap_scheme.yml
+++ b/data/other/ldap_scheme.yml
@@ -59,16 +59,16 @@ children:
             - groupOfNamesYnh
 
 depends_children:
-    cn=main.mail,ou=permission:
-        cn: main.mail
+    cn=mail.main,ou=permission:
+        cn: mail.main
         gidNumber: "5001"
         objectClass:
             - posixGroup
             - permissionYnh
         groupPermission:
             - "cn=all_users,ou=groups,dc=yunohost,dc=org"
-    cn=main.xmpp,ou=permission:
-        cn: main.xmpp
+    cn=xmpp.main,ou=permission:
+        cn: xmpp.main
         gidNumber: "5002"
         objectClass:
             - posixGroup
diff --git a/data/templates/dovecot/dovecot-ldap.conf b/data/templates/dovecot/dovecot-ldap.conf
index c7c9785fd..3a80ba47f 100644
--- a/data/templates/dovecot/dovecot-ldap.conf
+++ b/data/templates/dovecot/dovecot-ldap.conf
@@ -3,7 +3,7 @@ auth_bind = yes
 ldap_version = 3
 base = ou=users,dc=yunohost,dc=org
 user_attrs = uidNumber=500,gidNumber=8,mailuserquota=quota_rule=*:bytes=%$
-user_filter = (&(objectClass=inetOrgPerson)(uid=%n)(permission=cn=main.mail,ou=permission,dc=yunohost,dc=org))
-pass_filter = (&(objectClass=inetOrgPerson)(uid=%n)(permission=cn=main.mail,ou=permission,dc=yunohost,dc=org))
+user_filter = (&(objectClass=inetOrgPerson)(uid=%n)(permission=cn=mail.main,ou=permission,dc=yunohost,dc=org))
+pass_filter = (&(objectClass=inetOrgPerson)(uid=%n)(permission=cn=mail.main,ou=permission,dc=yunohost,dc=org))
 default_pass_scheme = SSHA
 
diff --git a/data/templates/metronome/domain.tpl.cfg.lua b/data/templates/metronome/domain.tpl.cfg.lua
index d523365db..e7f6bcef7 100644
--- a/data/templates/metronome/domain.tpl.cfg.lua
+++ b/data/templates/metronome/domain.tpl.cfg.lua
@@ -8,7 +8,7 @@ VirtualHost "{{ domain }}"
      hostname      = "localhost",
      user = {
        basedn        = "ou=users,dc=yunohost,dc=org",
-       filter        = "(&(objectClass=posixAccount)(mail=*@{{ domain }})(permission=cn=main.xmpp,ou=permission,dc=yunohost,dc=org))",
+       filter        = "(&(objectClass=posixAccount)(mail=*@{{ domain }})(permission=cn=xmpp.main,ou=permission,dc=yunohost,dc=org))",
        usernamefield = "mail",
        namefield     = "cn",
        },
diff --git a/data/templates/postfix/plain/ldap-accounts.cf b/data/templates/postfix/plain/ldap-accounts.cf
index 9f6f94e6d..75f38cf58 100644
--- a/data/templates/postfix/plain/ldap-accounts.cf
+++ b/data/templates/postfix/plain/ldap-accounts.cf
@@ -1,5 +1,5 @@
 server_host = localhost
 server_port = 389
 search_base = dc=yunohost,dc=org
-query_filter = (&(objectClass=mailAccount)(mail=%s)(permission=cn=main.mail,ou=permission,dc=yunohost,dc=org))
+query_filter = (&(objectClass=mailAccount)(mail=%s)(permission=cn=mail.main,ou=permission,dc=yunohost,dc=org))
 result_attribute = uid
diff --git a/data/templates/postfix/plain/ldap-aliases.cf b/data/templates/postfix/plain/ldap-aliases.cf
index 5e7d3a6c1..46563ae22 100644
--- a/data/templates/postfix/plain/ldap-aliases.cf
+++ b/data/templates/postfix/plain/ldap-aliases.cf
@@ -1,5 +1,5 @@
 server_host = localhost
 server_port = 389
 search_base = dc=yunohost,dc=org
-query_filter = (&(objectClass=mailAccount)(mail=%s)(permission=cn=main.mail,ou=permission,dc=yunohost,dc=org))
+query_filter = (&(objectClass=mailAccount)(mail=%s)(permission=cn=mail.main,ou=permission,dc=yunohost,dc=org))
 result_attribute = maildrop
diff --git a/src/yunohost/app.py b/src/yunohost/app.py
index 4a14c5e4b..105d4faf7 100644
--- a/src/yunohost/app.py
+++ b/src/yunohost/app.py
@@ -432,7 +432,7 @@ def app_map(app=None, raw=False, user=None):
         if user is not None:
             ldap = _get_ldap_interface()
             if not ldap.search(base='ou=permission,dc=yunohost,dc=org',
-                               filter='(&(objectclass=permissionYnh)(cn=main.%s)(inheritPermission=uid=%s,ou=users,dc=yunohost,dc=org))' % (app_id, user),
+                               filter='(&(objectclass=permissionYnh)(cn=%s.main)(inheritPermission=uid=%s,ou=users,dc=yunohost,dc=org))' % (app_id, user),
                                attrs=['cn']):
                 continue
 
diff --git a/src/yunohost/backup.py b/src/yunohost/backup.py
index 55b6678b8..9a27031ae 100644
--- a/src/yunohost/backup.py
+++ b/src/yunohost/backup.py
@@ -1247,7 +1247,7 @@ class RestoreManager():
 
         # Remove all permission for all app which sill in the LDAP
         for per in ldap.search('ou=permission,dc=yunohost,dc=org',
-                               '(&(objectClass=permissionYnh)(!(cn=main.mail))(!(cn=main.xmpp))(!(cn=main.sftp)))',
+                               '(&(objectClass=permissionYnh)(!(cn=mail.main))(!(cn=xmpp.main))(!(cn=sftp.main)))',
                                ['cn']):
             if not ldap.remove('cn=%s,ou=permission' % per['cn'][0]):
                 raise YunohostError('permission_deletion_failed',

From 34df84e222e04a975e0315e8155f5045ba9b9f0c Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Wed, 11 Sep 2019 00:57:32 +0200
Subject: [PATCH 03/45] group add -> group create, to be consistent with user
 create/delete

---
 data/actionsmap/yunohost.yml                         |  4 ++--
 locales/en.json                                      |  2 +-
 .../data_migrations/0011_setup_group_permission.py   |  4 ++--
 src/yunohost/tests/test_user-group.py                | 12 ++++++------
 src/yunohost/user.py                                 |  4 ++--
 5 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/data/actionsmap/yunohost.yml b/data/actionsmap/yunohost.yml
index cbb7756b0..8a6c10b5f 100644
--- a/data/actionsmap/yunohost.yml
+++ b/data/actionsmap/yunohost.yml
@@ -212,8 +212,8 @@ user:
                             help: fields to fetch
                             nargs: "+"
 
-                ### user_group_add()
-                add:
+                ### user_group_create()
+                create:
                     action_help: Create group
                     api: POST /users/groups
                     arguments:
diff --git a/locales/en.json b/locales/en.json
index be00d5b1e..815c40b75 100644
--- a/locales/en.json
+++ b/locales/en.json
@@ -288,7 +288,7 @@
     "log_regen_conf": "Regenerate system configurations '{}'",
     "log_user_create": "Add '{}' user",
     "log_user_delete": "Delete '{}' user",
-    "log_user_group_add": "Add '{}' group",
+    "log_user_group_create": "Create '{}' group",
     "log_user_group_delete": "Delete '{}' group",
     "log_user_group_update": "Update '{}' group",
     "log_user_update": "Update information of '{}' user",
diff --git a/src/yunohost/data_migrations/0011_setup_group_permission.py b/src/yunohost/data_migrations/0011_setup_group_permission.py
index 17fd8f4a5..9699d2cd9 100644
--- a/src/yunohost/data_migrations/0011_setup_group_permission.py
+++ b/src/yunohost/data_migrations/0011_setup_group_permission.py
@@ -7,7 +7,7 @@ from yunohost.utils.error import YunohostError
 from moulinette.utils.log import getActionLogger
 
 from yunohost.tools import Migration
-from yunohost.user import user_group_add, user_group_update
+from yunohost.user import user_group_create, user_group_update
 from yunohost.app import app_setting, app_list
 from yunohost.regenconf import regen_conf
 from yunohost.permission import permission_add, permission_sync_to_user
@@ -65,7 +65,7 @@ class MyMigration(Migration):
             username = user_info['uid'][0]
             ldap.update('uid=%s,ou=users' % username,
                         {'objectClass': ['mailAccount', 'inetOrgPerson', 'posixAccount', 'userPermissionYnh']})
-            user_group_add(username, gid=user_info['uidNumber'][0], sync_perm=False)
+            user_group_create(username, gid=user_info['uidNumber'][0], sync_perm=False)
             user_group_update(groupname=username, add_user=username, force=True, sync_perm=False)
             user_group_update(groupname='all_users', add_user=username, force=True, sync_perm=False)
 
diff --git a/src/yunohost/tests/test_user-group.py b/src/yunohost/tests/test_user-group.py
index 3973f0c7d..1defce466 100644
--- a/src/yunohost/tests/test_user-group.py
+++ b/src/yunohost/tests/test_user-group.py
@@ -1,7 +1,7 @@
 import pytest
 
 from moulinette.core import MoulinetteError
-from yunohost.user import user_list, user_info, user_group_list, user_create, user_delete, user_update, user_group_add, user_group_delete, user_group_update, user_group_info
+from yunohost.user import user_list, user_info, user_group_list, user_create, user_delete, user_update, user_group_create, user_group_delete, user_group_update, user_group_info
 from yunohost.domain import _get_maindomain
 from yunohost.utils.error import YunohostError
 from yunohost.tests.test_permission import check_LDAP_db_integrity
@@ -24,8 +24,8 @@ def setup_function(function):
     user_create("bob", "Bob", "Snow", "bob@" + maindomain, "test123Ynh")
     user_create("jack", "Jack", "Black", "jack@" + maindomain, "test123Ynh")
 
-    user_group_add("dev")
-    user_group_add("apps")
+    user_group_create("dev")
+    user_group_create("apps")
     user_group_update("dev", add_user=["alice"])
     user_group_update("apps", add_user=["bob"])
 
@@ -83,7 +83,7 @@ def test_del_user():
     assert "alice" not in group_res['all_users']['members']
 
 def test_add_group():
-    user_group_add("adminsys")
+    user_group_create("adminsys")
 
     group_res = user_group_list()['groups']
     assert "adminsys" in group_res
@@ -122,12 +122,12 @@ def test_del_bad_user_1():
 def test_add_bad_group_1():
     # Check groups already exist with special group "all_users"
     with pytest.raises(YunohostError):
-        user_group_add("all_users")
+        user_group_create("all_users")
 
 def test_add_bad_group_2():
     # Check groups already exist (for standard groups)
     with pytest.raises(MoulinetteError):
-        user_group_add("dev")
+        user_group_create("dev")
 
 def test_del_bad_group_1():
     # Check not allowed to remove this groups
diff --git a/src/yunohost/user.py b/src/yunohost/user.py
index a6c262ed7..e5480ca92 100644
--- a/src/yunohost/user.py
+++ b/src/yunohost/user.py
@@ -218,7 +218,7 @@ def user_create(operation_logger, username, firstname, lastname, mail, password,
                                exc_info=1)
 
         # Create group for user and add to group 'all_users'
-        user_group_add(groupname=username, gid=uid, sync_perm=False)
+        user_group_create(groupname=username, gid=uid, sync_perm=False)
         user_group_update(groupname=username, add_user=username, force=True, sync_perm=False)
         user_group_update(groupname='all_users', add_user=username, force=True, sync_perm=True)
 
@@ -554,7 +554,7 @@ def user_group_list(fields=None):
 
 
 @is_unit_operation([('groupname', 'user')])
-def user_group_add(operation_logger, groupname, gid=None, sync_perm=True):
+def user_group_create(operation_logger, groupname, gid=None, sync_perm=True):
     """
     Create group
 

From f60af2053f959d02fe958310ff9eaa40a9877be3 Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Wed, 11 Sep 2019 01:06:20 +0200
Subject: [PATCH 04/45] permission_add/remove becomes create/delete to be
 consistent with user and group create/delete. In the context of permissions,
 add/remove shall instead be related to adding/removing an existing permission
 for a user or group.

---
 data/helpers.d/setting                        |  4 ++--
 src/yunohost/app.py                           | 10 +++++-----
 src/yunohost/backup.py                        |  4 ++--
 .../0011_setup_group_permission.py            |  4 ++--
 src/yunohost/permission.py                    |  4 ++--
 src/yunohost/tests/test_permission.py         | 20 +++++++++----------
 6 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/data/helpers.d/setting b/data/helpers.d/setting
index da711b4bd..e002afa57 100644
--- a/data/helpers.d/setting
+++ b/data/helpers.d/setting
@@ -249,7 +249,7 @@ ynh_permission_create() {
     if [[ -n ${urls:-} ]]; then
         urls=",urls=['${urls//';'/"','"}']"
     fi
-    yunohost tools shell -c "from yunohost.permission import permission_add; permission_add('$app', '$permission' ${defaultdisallow:-} ${urls:-}, sync_perm=False)"
+    yunohost tools shell -c "from yunohost.permission import permission_create; permission_create('$app', '$permission' ${defaultdisallow:-} ${urls:-}, sync_perm=False)"
 }
 
 # Remove a permission for the app (note that when the app is removed all permission is automatically removed)
@@ -263,7 +263,7 @@ ynh_permission_remove() {
     local permission
     ynh_handle_getopts_args "$@"
 
-    yunohost tools shell -c "from yunohost.permission import permission_remove; permission_remove('$app', '$permission', sync_perm=False)"
+    yunohost tools shell -c "from yunohost.permission import permission_delete; permission_delete('$app', '$permission', sync_perm=False)"
 }
 
 # Add a path managed by the SSO
diff --git a/src/yunohost/app.py b/src/yunohost/app.py
index 105d4faf7..c2fb87acf 100644
--- a/src/yunohost/app.py
+++ b/src/yunohost/app.py
@@ -738,7 +738,7 @@ def app_install(operation_logger, app, label=None, args=None, no_remove_on_failu
     from yunohost.utils.ldap import _get_ldap_interface
     from yunohost.hook import hook_add, hook_remove, hook_exec, hook_callback
     from yunohost.log import OperationLogger
-    from yunohost.permission import permission_add, permission_update, permission_remove, permission_sync_to_user
+    from yunohost.permission import permission_create, permission_update, permission_delete, permission_sync_to_user
     ldap = _get_ldap_interface()
 
     # Fetch or extract sources
@@ -875,7 +875,7 @@ def app_install(operation_logger, app, label=None, args=None, no_remove_on_failu
     # Create permission before the install (useful if the install script redefine the permission)
     # Note that sync_perm is disabled to avoid triggering a whole bunch of code and messages
     # can't be sure that we don't have one case when it's needed
-    permission_add(app=app_instance_name, permission="main", sync_perm=False)
+    permission_create(app=app_instance_name, permission="main", sync_perm=False)
 
     # Execute the app install script
     install_retcode = 1
@@ -914,7 +914,7 @@ def app_install(operation_logger, app, label=None, args=None, no_remove_on_failu
                                     filter='(&(objectclass=permissionYnh)(cn=*.%s))' % app_instance_name, attrs=['cn'])
                 permission_list = [p['cn'][0] for p in result]
                 for l in permission_list:
-                    permission_remove(app_instance_name, l.split('.')[0], force=True)
+                    permission_delete(app_instance_name, l.split('.')[0], force=True)
 
                 if remove_retcode != 0:
                     msg = m18n.n('app_not_properly_removed',
@@ -980,7 +980,7 @@ def app_remove(operation_logger, app):
     """
     from yunohost.utils.ldap import _get_ldap_interface
     from yunohost.hook import hook_exec, hook_remove, hook_callback
-    from yunohost.permission import permission_remove, permission_sync_to_user
+    from yunohost.permission import permission_delete, permission_sync_to_user
     if not _is_installed(app):
         raise YunohostError('app_not_installed', app=app, all_apps=_get_all_installed_apps_id())
 
@@ -1031,7 +1031,7 @@ def app_remove(operation_logger, app):
                          filter='(&(objectclass=permissionYnh)(cn=*.%s))' % app, attrs=['cn'])
     permission_list = [p['cn'][0] for p in result]
     for l in permission_list:
-        permission_remove(app, l.split('.')[0], force=True, sync_perm=False)
+        permission_delete(app, l.split('.')[0], force=True, sync_perm=False)
 
     permission_sync_to_user()
 
diff --git a/src/yunohost/backup.py b/src/yunohost/backup.py
index 9a27031ae..9e90ece2a 100644
--- a/src/yunohost/backup.py
+++ b/src/yunohost/backup.py
@@ -1303,7 +1303,7 @@ class RestoreManager():
         """
         from moulinette.utils.filesystem import read_ldif
         from yunohost.user import user_group_list
-        from yunohost.permission import permission_remove
+        from yunohost.permission import permission_delete
         from yunohost.utils.ldap import _get_ldap_interface
         ldap = _get_ldap_interface()
 
@@ -1441,7 +1441,7 @@ class RestoreManager():
                                  filter='(&(objectclass=permissionYnh)(cn=*.%s))' % app_instance_name, attrs=['cn'])
             permission_list = [p['cn'][0] for p in result]
             for l in permission_list:
-                permission_remove(app_instance_name, l.split('.')[0], force=True)
+                permission_delete(app_instance_name, l.split('.')[0], force=True)
 
             # TODO Cleaning app hooks
         else:
diff --git a/src/yunohost/data_migrations/0011_setup_group_permission.py b/src/yunohost/data_migrations/0011_setup_group_permission.py
index 9699d2cd9..d0bae2d95 100644
--- a/src/yunohost/data_migrations/0011_setup_group_permission.py
+++ b/src/yunohost/data_migrations/0011_setup_group_permission.py
@@ -10,7 +10,7 @@ from yunohost.tools import Migration
 from yunohost.user import user_group_create, user_group_update
 from yunohost.app import app_setting, app_list
 from yunohost.regenconf import regen_conf
-from yunohost.permission import permission_add, permission_sync_to_user
+from yunohost.permission import permission_create, permission_sync_to_user
 from yunohost.user import user_permission_add
 
 logger = getActionLogger('yunohost.migration')
@@ -85,7 +85,7 @@ class MyMigration(Migration):
             domain = app_setting(app, 'domain')
 
             urls = [domain + path] if domain and path else None
-            permission_add(app, permission='main', urls=urls, default_allow=True, sync_perm=False)
+            permission_create(app, permission='main', urls=urls, default_allow=True, sync_perm=False)
             if permission:
                 allowed_group = permission.split(',')
                 user_permission_add([app], permission='main', group=allowed_group, sync_perm=False)
diff --git a/src/yunohost/permission.py b/src/yunohost/permission.py
index f7fa307da..3f9131018 100644
--- a/src/yunohost/permission.py
+++ b/src/yunohost/permission.py
@@ -317,7 +317,7 @@ def user_permission_clear(operation_logger, app=[], permission=None, sync_perm=T
 
 
 @is_unit_operation(['permission', 'app'])
-def permission_add(operation_logger, app, permission, urls=None, default_allow=True, sync_perm=True):
+def permission_create(operation_logger, app, permission, urls=None, default_allow=True, sync_perm=True):
     """
     Create a new permission for a specific application
 
@@ -431,7 +431,7 @@ def permission_update(operation_logger, app, permission, add_url=None, remove_ur
 
 
 @is_unit_operation(['permission', 'app'])
-def permission_remove(operation_logger, app, permission, force=False, sync_perm=True):
+def permission_delete(operation_logger, app, permission, force=False, sync_perm=True):
     """
     Remove a permission for a specific application
 
diff --git a/src/yunohost/tests/test_permission.py b/src/yunohost/tests/test_permission.py
index 3b9815f63..8222248b1 100644
--- a/src/yunohost/tests/test_permission.py
+++ b/src/yunohost/tests/test_permission.py
@@ -3,7 +3,7 @@ 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_create, user_permission_list, user_delete, user_group_list, user_group_delete, user_permission_add, user_permission_remove, user_permission_clear
-from yunohost.permission import permission_add, permission_update, permission_remove
+from yunohost.permission import permission_create, permission_update, permission_delete
 from yunohost.domain import _get_maindomain
 from yunohost.utils.error import YunohostError
 
@@ -21,15 +21,15 @@ def clean_user_groups_permission():
     for a, per in user_permission_list()['permissions'].items():
         if a in ['wiki', 'blog', 'site']:
             for p in per:
-                permission_remove(a, p, force=True, sync_perm=False)
+                permission_delete(a, p, force=True, sync_perm=False)
 
 def setup_function(function):
     clean_user_groups_permission()
 
     user_create("alice", "Alice", "White", "alice@" + maindomain, "test123Ynh")
     user_create("bob", "Bob", "Snow", "bob@" + maindomain, "test123Ynh")
-    permission_add("wiki", "main", [maindomain + "/wiki"], sync_perm=False)
-    permission_add("blog", "main", sync_perm=False)
+    permission_create("wiki", "main", [maindomain + "/wiki"], sync_perm=False)
+    permission_create("blog", "main", sync_perm=False)
 
     user_permission_add(["blog"], "main", group="alice")
 
@@ -177,7 +177,7 @@ def test_list_permission():
 #
 
 def test_add_permission_1():
-    permission_add("site", "test")
+    permission_create("site", "test")
 
     res = user_permission_list()['permissions']
     assert "site" in res
@@ -186,7 +186,7 @@ def test_add_permission_1():
     assert set(["alice", "bob"]) == set(res['site']['test']['allowed_users'])
 
 def test_add_permission_2():
-    permission_add("site", "main", default_allow=False)
+    permission_create("site", "main", default_allow=False)
 
     res = user_permission_list()['permissions']
     assert "site" in res
@@ -195,7 +195,7 @@ def test_add_permission_2():
     assert [] == res['site']['main']['allowed_users']
 
 def test_remove_permission():
-    permission_remove("wiki", "main", force=True)
+    permission_delete("wiki", "main", force=True)
 
     res = user_permission_list()['permissions']
     assert "wiki" not in res
@@ -207,12 +207,12 @@ def test_remove_permission():
 def test_add_bad_permission():
     # Create permission with same name
     with pytest.raises(YunohostError):
-        permission_add("wiki", "main")
+        permission_create("wiki", "main")
 
 def test_remove_bad_permission():
     # Remove not existant permission
     with pytest.raises(MoulinetteError):
-        permission_remove("non_exit", "main", force=True)
+        permission_delete("non_exit", "main", force=True)
 
     res = user_permission_list()['permissions']
     assert "wiki" in res
@@ -226,7 +226,7 @@ def test_remove_bad_permission():
 
 def test_remove_main_permission():
     with pytest.raises(YunohostError):
-        permission_remove("blog", "main")
+        permission_delete("blog", "main")
 
     res = user_permission_list()['permissions']
     assert "mail" in res

From a6d68c76c4eaa998db3fa77e7ece0c811e85e8ad Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Wed, 11 Sep 2019 01:31:22 +0200
Subject: [PATCH 05/45] permission_update -> permission_urls (+ tweak the
 helper name) so that it's more differentiable from user_permission_update

---
 data/helpers.d/setting     | 10 +++++-----
 src/yunohost/permission.py | 18 ++++++++++++++++--
 2 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/data/helpers.d/setting b/data/helpers.d/setting
index e002afa57..e6311fc1f 100644
--- a/data/helpers.d/setting
+++ b/data/helpers.d/setting
@@ -268,18 +268,18 @@ ynh_permission_remove() {
 
 # Add a path managed by the SSO
 #
-# usage: ynh_permission_add_path --app "app" --permission "permission" --url "url" ["url" ...]
+# usage: ynh_permission_add_url --app "app" --permission "permission" --url "url" ["url" ...]
 # | arg: app - the application id
 # | arg: permission - the name for the permission
 # | arg: url - the FULL url for the the permission (ex domain.tld/apps/admin)
-ynh_permission_add_path() {
+ynh_permission_add_url() {
     declare -Ar args_array=( [a]=app= [p]=permission= [u]=url= )
     local app
     local permission
     local url
     ynh_handle_getopts_args "$@"
 
-    yunohost tools shell -c "from yunohost.permission import permission_update; permission_update('$app', '$permission', add_url=['${url//';'/"','"}'], sync_perm=False)"
+    yunohost tools shell -c "from yunohost.permission import permission_urls; permission_urls('$app', '$permission', add_url=['${url//';'/"','"}'], sync_perm=False)"
 }
 
 # Remove a path managed by the SSO
@@ -288,12 +288,12 @@ ynh_permission_add_path() {
 # | arg: app - the application id
 # | arg: permission - the name for the permission
 # | arg: url - the FULL url for the the permission (ex domain.tld/apps/admin)
-ynh_permission_del_path() {
+ynh_permission_remove_url() {
     declare -Ar args_array=( [a]=app= [p]=permission= [u]=url= )
     local app
     local permission
     local url
     ynh_handle_getopts_args "$@"
 
-    yunohost tools shell -c "from yunohost.permission import permission_update; permission_update('$app', '$permission', remove_url=['${url//';'/"','"}'], sync_perm=False)"
+    yunohost tools shell -c "from yunohost.permission import permission_urls; permission_urls('$app', '$permission', remove_url=['${url//';'/"','"}'], sync_perm=False)"
 }
diff --git a/src/yunohost/permission.py b/src/yunohost/permission.py
index 3f9131018..1a5465674 100644
--- a/src/yunohost/permission.py
+++ b/src/yunohost/permission.py
@@ -35,6 +35,12 @@ from yunohost.log import is_unit_operation
 
 logger = getActionLogger('yunohost.user')
 
+#
+#
+#  The followings are the methods exposed through the "yunohost user permission" interface
+#
+#
+
 
 def user_permission_list(app=None, permission=None, username=None, group=None):
     """
@@ -315,6 +321,14 @@ def user_permission_clear(operation_logger, app=[], permission=None, sync_perm=T
 
     return user_permission_list(app, permission)
 
+#
+#
+#  The followings methods are *not* directly exposed.
+#  They are used to create/delete the permissions (e.g. during app install/remove)
+#  and by some app helpers to possibly add additional permissions and tweak the urls
+#
+#
+
 
 @is_unit_operation(['permission', 'app'])
 def permission_create(operation_logger, app, permission, urls=None, default_allow=True, sync_perm=True):
@@ -374,9 +388,9 @@ def permission_create(operation_logger, app, permission, urls=None, default_allo
 
 
 @is_unit_operation(['permission', 'app'])
-def permission_update(operation_logger, app, permission, add_url=None, remove_url=None, sync_perm=True):
+def permission_urls(operation_logger, app, permission, add_url=None, remove_url=None, sync_perm=True):
     """
-    Update a permission for a specific application
+    Update urls related to a permission for a specific application
 
     Keyword argument:
         app            -- an application OR sftp, xmpp (metronome), mail

From 0f7b8c3515e80afe648d55445517db60d848f609 Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Wed, 11 Sep 2019 02:08:46 +0200
Subject: [PATCH 06/45] Simplify group list interface and code

---
 data/actionsmap/yunohost.yml | 11 ++++--
 src/yunohost/backup.py       |  2 +-
 src/yunohost/permission.py   | 12 +++---
 src/yunohost/user.py         | 76 +++++++++++++-----------------------
 src/yunohost/utils/ldap.py   | 14 +++++++
 5 files changed, 55 insertions(+), 60 deletions(-)

diff --git a/data/actionsmap/yunohost.yml b/data/actionsmap/yunohost.yml
index 8a6c10b5f..b463df646 100644
--- a/data/actionsmap/yunohost.yml
+++ b/data/actionsmap/yunohost.yml
@@ -208,9 +208,14 @@ user:
                     action_help: List group
                     api: GET /users/groups
                     arguments:
-                        --fields:
-                            help: fields to fetch
-                            nargs: "+"
+                        -n:
+                            full: --names-only
+                            help: Only list the name of the groups without any additional info
+                            action: store_true
+                        -f:
+                            full: --full
+                            help: List all the info available for each groups
+                            action: store_true
 
                 ### user_group_create()
                 create:
diff --git a/src/yunohost/backup.py b/src/yunohost/backup.py
index 9e90ece2a..f96146ea0 100644
--- a/src/yunohost/backup.py
+++ b/src/yunohost/backup.py
@@ -1374,7 +1374,7 @@ class RestoreManager():
                 filtred_entries =  ['entryUUID', 'creatorsName', 'createTimestamp', 'entryCSN', 'structuralObjectClass',
                                     'modifiersName', 'modifyTimestamp', 'inheritPermission', 'memberUid']
                 entries = read_ldif('%s/permission.ldif' % app_settings_in_archive, filtred_entries)
-                group_list = user_group_list(['cn'])['groups']
+                group_list = user_group_list()['groups']
                 for dn, entry in entries:
                     # Remove the group which has been removed
                     for group in entry['groupPermission']:
diff --git a/src/yunohost/permission.py b/src/yunohost/permission.py
index 1a5465674..0b77a3e5c 100644
--- a/src/yunohost/permission.py
+++ b/src/yunohost/permission.py
@@ -169,13 +169,13 @@ def user_permission_update(operation_logger, app=[], permission=None, add_userna
 
     # Validate that the group exist
     for g in add_group:
-        if g not in user_group_list(['cn'])['groups']:
+        if g not in user_group_list()['groups']:
             raise YunohostError('group_unknown', group=g)
     for u in add_username:
         if u not in user_list(['uid'])['users']:
             raise YunohostError('user_unknown', user=u)
     for g in del_group:
-        if g not in user_group_list(['cn'])['groups']:
+        if g not in user_group_list()['groups']:
             raise YunohostError('group_unknown', group=g)
     for u in del_username:
         if u not in user_list(['uid'])['users']:
@@ -244,14 +244,12 @@ def user_permission_update(operation_logger, app=[], permission=None, add_userna
     for a in app:
         allowed_users = set()
         disallowed_users = set()
-        group_list = user_group_list(['member'])['groups']
+        group_list = user_group_list()['groups']
 
         for g in add_group:
-            if 'members' in group_list[g]:
-                allowed_users.union(group_list[g]['members'])
+            allowed_users.union(group_list[g]['members'])
         for g in del_group:
-            if 'members' in group_list[g]:
-                disallowed_users.union(group_list[g]['members'])
+            disallowed_users.union(group_list[g]['members'])
 
         allowed_users = ','.join(allowed_users)
         disallowed_users = ','.join(disallowed_users)
diff --git a/src/yunohost/user.py b/src/yunohost/user.py
index e5480ca92..787058fbc 100644
--- a/src/yunohost/user.py
+++ b/src/yunohost/user.py
@@ -489,66 +489,44 @@ def user_info(username):
 #
 # Group subcategory
 #
-def user_group_list(fields=None):
+def user_group_list(names_only=False, full=False):
     """
     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
-
+        names-only -- Only list the name of the groups without any additional info
+        full -- List all the info available for each groups
     """
-    from yunohost.utils.ldap import _get_ldap_interface
+
+    # Fetch relevant informations
+
+    from yunohost.utils.ldap import _get_ldap_interface, _ldap_path_extract
     ldap = _get_ldap_interface()
-    group_attr = {
-        'cn': 'groupname',
-        'member': 'members',
-        'permission': 'permission'
-    }
-    attrs = ['cn']
-    groups = {}
 
-    if fields:
-        keys = group_attr.keys()
-        for attr in fields:
-            if attr in keys:
-                attrs.append(attr)
-            else:
-                raise YunohostError('field_invalid', attr)
+    if names_only:
+        fields_to_fetch = ["cn"]
+    elif full:
+        fields_to_fetch = ["cn", "member", "permission"]
     else:
-        attrs = ['cn', 'member']
+        fields_to_fetch = ["cn", "member"]
 
-    result = ldap.search('ou=groups,dc=yunohost,dc=org',
-                         '(objectclass=groupOfNamesYnh)',
-                         attrs)
+    groups_infos = ldap.search('ou=groups,dc=yunohost,dc=org',
+                               '(objectclass=groupOfNamesYnh)',
+                               fields_to_fetch)
 
-    for group in result:
-        # The group "admins" should be hidden for the user
-        if group_attr['cn'] == "admins":
-            continue
-        entry = {}
-        for attr, values in group.items():
-            if values:
-                if attr == "member":
-                    entry[group_attr[attr]] = []
-                    for v in values:
-                        entry[group_attr[attr]].append(v.split("=")[1].split(",")[0])
-                elif attr == "permission":
-                    entry[group_attr[attr]] = {}
-                    for v in values:
-                        permission = v.split("=")[1].split(",")[0].split(".")[1]
-                        pType = v.split("=")[1].split(",")[0].split(".")[0]
-                        if permission in entry[group_attr[attr]]:
-                            entry[group_attr[attr]][permission].append(pType)
-                        else:
-                            entry[group_attr[attr]][permission] = [pType]
-                else:
-                    entry[group_attr[attr]] = values[0]
+    # Parse / organize information to be outputed
 
-        groupname = entry[group_attr['cn']]
-        groups[groupname] = entry
+    groups = {}
+    for infos in groups_infos:
+        name = infos["cn"][0]
+        groups[name] = {}
+        if "member" in fields_to_fetch:
+            groups[name]["members"] = [_ldap_path_extract(p, "uid") for p in infos.get("member", [])]
+        if "permission" in fields_to_fetch:
+            groups[name]["permissions"] = [_ldap_path_extract(p, "cn") for p in infos.get("permission", [])]
+
+    if names_only:
+        groups = groups.keys()
 
     return {'groups': groups}
 
diff --git a/src/yunohost/utils/ldap.py b/src/yunohost/utils/ldap.py
index 186cdbdec..c3b5065a1 100644
--- a/src/yunohost/utils/ldap.py
+++ b/src/yunohost/utils/ldap.py
@@ -40,6 +40,20 @@ def _get_ldap_interface():
 
     return _ldap_interface
 
+
+# We regularly want to extract stuff like 'bar' in ldap path like
+# foo=bar,dn=users.example.org,ou=example.org,dc=org so this small helper allow
+# to do this without relying of dozens of mysterious string.split()[0]
+#
+# e.g. using _ldap_path_extract(path, "foo") on the previous example will
+# return bar
+
+def _ldap_path_extract(path, info):
+    for element in path.split(","):
+        if element.startswith(info + "="):
+            return element[len(info + "="):]
+
+
 # Add this to properly close / delete the ldap interface / authenticator
 # when Python exits ...
 # Otherwise there's a risk that some funky error appears at the very end

From c5d0a270980188a0a2d04f8f6aece63a727d4206 Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Wed, 11 Sep 2019 03:00:29 +0200
Subject: [PATCH 07/45] Simplify group info and group update interface and code

---
 data/actionsmap/yunohost.yml                  |   8 +-
 .../0011_setup_group_permission.py            |   4 +-
 src/yunohost/tests/test_user-group.py         |  20 ++--
 src/yunohost/user.py                          | 101 ++++++++----------
 4 files changed, 58 insertions(+), 75 deletions(-)

diff --git a/data/actionsmap/yunohost.yml b/data/actionsmap/yunohost.yml
index b463df646..e727b87ef 100644
--- a/data/actionsmap/yunohost.yml
+++ b/data/actionsmap/yunohost.yml
@@ -249,15 +249,15 @@ user:
                             extra:
                                 pattern: *pattern_groupname
                         -a:
-                            full: --add-user
-                            help: User to add in group
+                            full: --add
+                            help: User(s) to add in the group
                             nargs: "*"
                             metavar: USERNAME
                             extra:
                                 pattern: *pattern_username
                         -r:
-                            full: --remove-user
-                            help: User to remove in group
+                            full: --remove
+                            help: User(s) to remove in the group
                             nargs: "*"
                             metavar: USERNAME
                             extra:
diff --git a/src/yunohost/data_migrations/0011_setup_group_permission.py b/src/yunohost/data_migrations/0011_setup_group_permission.py
index d0bae2d95..d2924f0af 100644
--- a/src/yunohost/data_migrations/0011_setup_group_permission.py
+++ b/src/yunohost/data_migrations/0011_setup_group_permission.py
@@ -66,8 +66,8 @@ class MyMigration(Migration):
             ldap.update('uid=%s,ou=users' % username,
                         {'objectClass': ['mailAccount', 'inetOrgPerson', 'posixAccount', 'userPermissionYnh']})
             user_group_create(username, gid=user_info['uidNumber'][0], sync_perm=False)
-            user_group_update(groupname=username, add_user=username, force=True, sync_perm=False)
-            user_group_update(groupname='all_users', add_user=username, force=True, sync_perm=False)
+            user_group_update(groupname=username, add=username, force=True, sync_perm=False)
+            user_group_update(groupname='all_users', add=username, force=True, sync_perm=False)
 
 
     def migrate_app_permission(self, app=None):
diff --git a/src/yunohost/tests/test_user-group.py b/src/yunohost/tests/test_user-group.py
index 1defce466..34e515ea0 100644
--- a/src/yunohost/tests/test_user-group.py
+++ b/src/yunohost/tests/test_user-group.py
@@ -26,8 +26,8 @@ def setup_function(function):
 
     user_group_create("dev")
     user_group_create("apps")
-    user_group_update("dev", add_user=["alice"])
-    user_group_update("apps", add_user=["bob"])
+    user_group_update("dev", add=["alice"])
+    user_group_update("apps", add=["bob"])
 
 def teardown_function(function):
     clean_user_groups()
@@ -151,28 +151,28 @@ def test_update_user_1():
     assert "NewLast" == info['lastname']
 
 def test_update_group_1():
-    user_group_update("dev", add_user=["bob"])
+    user_group_update("dev", add=["bob"])
 
     group_res = user_group_list()['groups']
     assert set(["alice", "bob"]) == set(group_res['dev']['members'])
 
 def test_update_group_2():
     # Try to add a user in a group when the user is already in
-    user_group_update("apps", add_user=["bob"])
+    user_group_update("apps", add=["bob"])
 
     group_res = user_group_list()['groups']
     assert ["bob"] == group_res['apps']['members']
 
 def test_update_group_3():
     # Try to remove a user in a group
-    user_group_update("apps", remove_user=["bob"])
+    user_group_update("apps", remove=["bob"])
 
     group_res = user_group_list()['groups']
     assert "members" not in group_res['apps']
 
 def test_update_group_4():
     # Try to remove a user in a group when it is not already in
-    user_group_update("apps", remove_user=["jack"])
+    user_group_update("apps", remove=["jack"])
 
     group_res = user_group_list()['groups']
     assert ["bob"] == group_res['apps']['members']
@@ -190,21 +190,21 @@ def test_bad_update_user_1():
 def bad_update_group_1():
     # Check groups not found
     with pytest.raises(YunohostError):
-        user_group_update("not_exit", add_user=["alice"])
+        user_group_update("not_exit", add=["alice"])
 
 def test_bad_update_group_2():
     # Check remove user in groups "all_users" not allowed
     with pytest.raises(YunohostError):
-        user_group_update("all_users", remove_user=["alice"])
+        user_group_update("all_users", remove=["alice"])
 
 def test_bad_update_group_3():
     # Check remove user in it own group not allowed
     with pytest.raises(YunohostError):
-        user_group_update("alice", remove_user=["alice"])
+        user_group_update("alice", remove=["alice"])
 
 def test_bad_update_group_1():
     # Check add bad user in group
     with pytest.raises(YunohostError):
-        user_group_update("dev", add_user=["not_exist"])
+        user_group_update("dev", add=["not_exist"])
 
     assert "not_exist" not in user_group_list()["groups"]["dev"]
diff --git a/src/yunohost/user.py b/src/yunohost/user.py
index 787058fbc..a1a671ccf 100644
--- a/src/yunohost/user.py
+++ b/src/yunohost/user.py
@@ -32,6 +32,7 @@ import crypt
 import random
 import string
 import subprocess
+import copy
 
 from moulinette import m18n
 from yunohost.utils.error import YunohostError
@@ -219,8 +220,8 @@ def user_create(operation_logger, username, firstname, lastname, mail, password,
 
         # Create group for user and add to group 'all_users'
         user_group_create(groupname=username, gid=uid, sync_perm=False)
-        user_group_update(groupname=username, add_user=username, force=True, sync_perm=False)
-        user_group_update(groupname='all_users', add_user=username, force=True, sync_perm=True)
+        user_group_update(groupname=username, add=username, force=True, sync_perm=False)
+        user_group_update(groupname='all_users', add=username, force=True, sync_perm=True)
 
         # TODO: Send a welcome mail to user
         logger.success(m18n.n('user_created'))
@@ -610,84 +611,67 @@ def user_group_delete(operation_logger, groupname, force=False, sync_perm=True):
 
 
 @is_unit_operation([('groupname', 'user')])
-def user_group_update(operation_logger, groupname, add_user=None, remove_user=None, force=False, sync_perm=True):
+def user_group_update(operation_logger, groupname, add=None, remove=None, force=False, sync_perm=True):
     """
     Update user informations
 
     Keyword argument:
         groupname -- Groupname to update
-        add_user -- User to add in group
-        remove_user -- User to remove in group
+        add -- User(s) to add in group
+        remove -- User(s) to remove in group
 
     """
 
     from yunohost.permission import permission_sync_to_user
     from yunohost.utils.ldap import _get_ldap_interface
 
+    # FIXME : we should also refuse to edit the main group of a user (e.g. group 'sam' related to user 'sam')
+
     if (groupname == 'all_users' or groupname == 'admins') and not force:
         raise YunohostError('edit_group_not_allowed', group=groupname)
 
     ldap = _get_ldap_interface()
 
-    # Populate group informations
-    attrs_to_fetch = ['member']
-    result = ldap.search(base='ou=groups,dc=yunohost,dc=org',
-                         filter='cn=' + groupname, attrs=attrs_to_fetch)
-    if not result:
-        raise YunohostError('group_unknown', group=groupname)
-    group = result[0]
+    # We extract the uid for each member of the group to keep a simple flat list of members
+    current_group = user_group_info(groupname)["members"]
+    new_group = copy.copy(current_group)
 
-    new_group_list = {'member': set(), 'memberUid': set()}
-    if 'member' in group:
-        new_group_list['member'] = set(group['member'])
-    else:
-        group['member'] = []
+    existing_users = user_list()['users'].keys()
 
-    existing_users = user_list(fields=['uid'])['users'].keys()
+    if add:
+        users_to_add = [add] if not isinstance(add, list) else add
 
-    if add_user:
-        if not isinstance(add_user, list):
-            add_user = [add_user]
-
-        for user in add_user:
+        for user in users_to_add:
             if user not in existing_users:
                 raise YunohostError('user_unknown', user=user)
 
-        for user in add_user:
-            userDN = "uid=" + user + ",ou=users,dc=yunohost,dc=org"
-            if userDN in group['member']:
+            if user in current_group:
                 logger.warning(m18n.n('user_already_in_group', user=user, group=groupname))
-            new_group_list['member'].add(userDN)
 
-    if remove_user:
-        if not isinstance(remove_user, list):
-            remove_user = [remove_user]
+        new_group += users_to_add
 
-        for user in remove_user:
+    if remove:
+        users_to_remove = [remove] if not isinstance(remove, list) else remove
+
+        for user in users_to_remove:
             if user == groupname:
+                # FIXME : well if the user equals the group, why pass the two info...
+                # anyway we should just forbid this from the very beginning ... (editing a user-related group)
                 raise YunohostError('remove_user_of_group_not_allowed', user=user, group=groupname)
 
-        for user in remove_user:
-            userDN = "uid=" + user + ",ou=users,dc=yunohost,dc=org"
-            if 'member' in group and userDN in group['member']:
-                new_group_list['member'].remove(userDN)
-            else:
+            if user not in current_group:
                 logger.warning(m18n.n('user_not_in_group', user=user, group=groupname))
 
-    # Sychronise memberUid with member (to keep the posix group structure)
-    # In posixgroup the main group of each user is only written in the gid number of the user
-    for member in new_group_list['member']:
-        member_Uid = member.split("=")[1].split(",")[0]
-        # Don't add main user in the group.
-        # Note that in the Unix system the main user of the group is linked by the gid in the user attribute.
-        # So the main user need to be not in the memberUid list of his own group.
-        if member_Uid != groupname:
-            new_group_list['memberUid'].add(member_Uid)
+        # Remove users_to_remove from new_group
+        # Kinda like a new_group -= users_to_remove
+        new_group = [u for u in new_group if u not in users_to_remove]
+
+    new_group_dns = ["uid=" + user + ",ou=users,dc=yunohost,dc=org" for user in new_group]
 
     operation_logger.start()
 
-    if new_group_list['member'] != set(group['member']):
-        if not ldap.update('cn=%s,ou=groups' % groupname, new_group_list):
+    if set(new_group) != set(current_group):
+        if not ldap.update('cn=%s,ou=groups' % groupname, {"member": set(new_group_dns), "memberUid": set(new_group)}):
             raise YunohostError('group_update_failed', group=groupname)
 
     logger.success(m18n.n('group_updated', group=groupname))
@@ -705,26 +689,25 @@ def user_group_info(groupname):
 
     """
 
-    from yunohost.utils.ldap import _get_ldap_interface
+    from yunohost.utils.ldap import _get_ldap_interface, _ldap_path_extract
     ldap = _get_ldap_interface()
 
-    group_attrs = [
-        'cn', 'member', 'permission'
-    ]
-    result = ldap.search('ou=groups,dc=yunohost,dc=org', "cn=" + groupname, group_attrs)
+    # Fetch info for this group
+    result = ldap.search('ou=groups,dc=yunohost,dc=org',
+                         "cn=" + groupname,
+                         ["cn", "member", "permission"])
 
     if not result:
         raise YunohostError('group_unknown', group=groupname)
 
-    group = result[0]
+    infos = result[0]
 
-    result_dict = {
-        'groupname': group['cn'][0],
-        'member': None
+    # Format data
+
+    return {
+        'members': [_ldap_path_extract(p, "uid") for p in infos.get("member", [])],
+        'permissions': [_ldap_path_extract(p, "cn") for p in infos.get("permission", [])]
     }
-    if 'member' in group:
-        result_dict['member'] = {m.split("=")[1].split(",")[0] for m in group['member']}
-    return result_dict
 
 
 #

From 97c637f44c4b3d1d2625c1c34bdeece51ace0ea4 Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Wed, 11 Sep 2019 03:00:59 +0200
Subject: [PATCH 08/45] Fix group command descriptions in the actionmap

---
 data/actionsmap/yunohost.yml | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/data/actionsmap/yunohost.yml b/data/actionsmap/yunohost.yml
index e727b87ef..d4940c043 100644
--- a/data/actionsmap/yunohost.yml
+++ b/data/actionsmap/yunohost.yml
@@ -201,11 +201,11 @@ user:
 
     subcategories:
         group:
-            subcategory_help: Manage group
+            subcategory_help: Manage user groups
             actions:
                 ### user_group_list()
                 list:
-                    action_help: List group
+                    action_help: List existing groups
                     api: GET /users/groups
                     arguments:
                         -n:
@@ -223,7 +223,7 @@ user:
                     api: POST /users/groups
                     arguments:
                         groupname:
-                            help: The unique group name to add
+                            help: Name of the group to be created
                             extra:
                                 pattern: &pattern_groupname
                                     - !!str ^[a-z0-9_]+$
@@ -235,7 +235,7 @@ user:
                     api: DELETE /users/groups/<groupname>
                     arguments:
                         groupname:
-                            help: Username to delete
+                            help: Name of the group to be deleted
                             extra:
                                 pattern: *pattern_groupname
 
@@ -245,7 +245,7 @@ user:
                     api: PUT /users/groups/<groupname>
                     arguments:
                         groupname:
-                            help: Username to update
+                            help: Name of the group to be updated
                             extra:
                                 pattern: *pattern_groupname
                         -a:
@@ -265,11 +265,11 @@ user:
 
                 ### user_group_info()
                 info:
-                    action_help: Get group information
+                    action_help: Get information for a specific group
                     api: GET /users/groups/<groupname>
                     arguments:
                         groupname:
-                            help: Groupname to get information
+                            help: Name of the group to get info about
                             extra:
                                 pattern: *pattern_username
 

From 112976f8ee17e1ebb0c74edb2fbe5c9dc6441fa3 Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Wed, 11 Sep 2019 03:30:51 +0200
Subject: [PATCH 09/45] Refuse to edit user primary groups

---
 locales/en.json      |  5 ++---
 src/yunohost/user.py | 43 ++++++++++++++++++++++++-------------------
 2 files changed, 26 insertions(+), 22 deletions(-)

diff --git a/locales/en.json b/locales/en.json
index 815c40b75..d3abf4fd0 100644
--- a/locales/en.json
+++ b/locales/en.json
@@ -196,7 +196,6 @@
     "dyndns_registration_failed": "Unable to register DynDNS domain: {error:s}",
     "dyndns_domain_not_provided": "Dyndns provider {provider:s} cannot provide domain {domain:s}.",
     "dyndns_unavailable": "Domain {domain:s} is not available.",
-    "edit_group_not_allowed": "You are not allowed to edit the group {group:s}",
     "edit_permission_with_group_all_users_not_allowed": "You are not allowed to edit permission for group 'all_users', use 'yunohost user permission clear APP' or 'yunohost user permission add APP -u USER' instead.",
     "error_when_removing_sftpuser_group": "Error when trying remove sftpusers group",
     "executing_command": "Executing command '{command:s}'…",
@@ -235,9 +234,10 @@
     "group_name_already_exist": "Group {name:s} already exist",
     "group_created": "Group '{group}' successfully created",
     "group_creation_failed": "Group creation failed for group '{group}'",
+    "group_cannot_be_edited": "The group {group:s} cannot be edited manually.",
+    "group_cannot_be_deleted": "The group {group:s} cannot be deleted manually.",
     "group_deleted": "Group '{group}' deleted",
     "group_deletion_failed": "Group '{group} 'deletion failed",
-    "group_deletion_not_allowed": "The group {group:s} cannot be deleted manually.",
     "group_info_failed": "Group info failed",
     "group_unknown": "Group {group:s} unknown",
     "group_updated": "Group '{group}' updated",
@@ -449,7 +449,6 @@
     "port_unavailable": "Port {port:d} is not available",
     "recommend_to_add_first_user": "The post-install is finished but YunoHost needs at least one user to work correctly, you should add one using 'yunohost user create <username>' or the admin interface.",
     "remove_main_permission_not_allowed": "Removing the main permission is not allowed",
-    "remove_user_of_group_not_allowed": "You are not allowed to remove the user {user:s} in the group {group:s}",
     "regenconf_file_backed_up": "The configuration file '{conf}' has been backed up to '{backup}'",
     "regenconf_file_copy_failed": "Unable to copy the new configuration file '{new}' to '{conf}'",
     "regenconf_file_kept_back": "The configuration file '{conf}' is expected to be deleted by regen-conf (category {category}) but has been kept back.",
diff --git a/src/yunohost/user.py b/src/yunohost/user.py
index a1a671ccf..8427cbd42 100644
--- a/src/yunohost/user.py
+++ b/src/yunohost/user.py
@@ -219,8 +219,7 @@ def user_create(operation_logger, username, firstname, lastname, mail, password,
                                exc_info=1)
 
         # Create group for user and add to group 'all_users'
-        user_group_create(groupname=username, gid=uid, sync_perm=False)
-        user_group_update(groupname=username, add=username, force=True, sync_perm=False)
+        user_group_create(groupname=username, gid=uid, primary_group=True, sync_perm=False)
         user_group_update(groupname='all_users', add=username, force=True, sync_perm=True)
 
         # TODO: Send a welcome mail to user
@@ -533,7 +532,7 @@ def user_group_list(names_only=False, full=False):
 
 
 @is_unit_operation([('groupname', 'user')])
-def user_group_create(operation_logger, groupname, gid=None, sync_perm=True):
+def user_group_create(operation_logger, groupname, gid=None, primary_group=False, sync_perm=True):
     """
     Create group
 
@@ -575,6 +574,12 @@ def user_group_create(operation_logger, groupname, gid=None, sync_perm=True):
         'gidNumber': gid,
     }
 
+    # Here we handle the creation of a primary group
+    # We want to initialize this group to contain the corresponding user
+    # (then we won't be able to add/remove any user in this group)
+    if primary_group:
+        attr_dict["member"] = ["uid=" + groupname + ",ou=users,dc=yunohost,dc=org"]
+
     if ldap.add('cn=%s,ou=groups' % groupname, attr_dict):
         logger.success(m18n.n('group_created', group=groupname))
         if sync_perm:
@@ -596,9 +601,14 @@ 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
 
-    forbidden_groups = ["all_users", "admins"] + user_list(fields=['uid'])['users'].keys()
-    if not force and groupname in forbidden_groups:
-        raise YunohostError('group_deletion_not_allowed', group=groupname)
+    # Refuse to delete primary groups of a user (e.g. group 'sam' related to user 'sam')
+    # without the force option...
+    #
+    # We also can't delete "all_users" because that's a special group...
+    existing_users = user_list()['users'].keys()
+    undeletable_groups = existing_users + ["all_users", "admins"]
+    if groupname in undeletable_groups and not force:
+        raise YunohostError('group_cannot_be_deleted', group=groupname)
 
     operation_logger.start()
     ldap = _get_ldap_interface()
@@ -625,19 +635,18 @@ def user_group_update(operation_logger, groupname, add=None, remove=None, force=
     from yunohost.permission import permission_sync_to_user
     from yunohost.utils.ldap import _get_ldap_interface
 
-    # FIXME : we should also refuse to edit the main group of a user (e.g. group 'sam' related to user 'sam')
-
-    if (groupname == 'all_users' or groupname == 'admins') and not force:
-        raise YunohostError('edit_group_not_allowed', group=groupname)
-
-    ldap = _get_ldap_interface()
+    # Refuse to edit a primary group of a user (e.g. group 'sam' related to user 'sam')
+    # Those kind of group should only ever contain the user (e.g. sam) and only this one.
+    # We also can't edit "all_users" without the force option because that's a special group...
+    existing_users = user_list()['users'].keys()
+    uneditable_groups = existing_users + ["all_users", "admins"]
+    if groupname in uneditable_groups and not force:
+        raise YunohostError('group_cannot_be_edited', group=groupname)
 
     # We extract the uid for each member of the group to keep a simple flat list of members
     current_group = user_group_info(groupname)["members"]
     new_group = copy.copy(current_group)
 
-    existing_users = user_list()['users'].keys()
-
     if add:
         users_to_add = [add] if not isinstance(add, list) else add
 
@@ -654,11 +663,6 @@ def user_group_update(operation_logger, groupname, add=None, remove=None, force=
         users_to_remove = [remove] if not isinstance(remove, list) else remove
 
         for user in users_to_remove:
-            if user == groupname:
-                # FIXME : well if the user equals the group, why pass the two info...
-                # anyway we should just forbid this from the very beginning ... (editing a user-related group)
-                raise YunohostError('remove_user_of_group_not_allowed', user=user, group=groupname)
-
             if user not in current_group:
                 logger.warning(m18n.n('user_not_in_group', user=user, group=groupname))
 
@@ -671,6 +675,7 @@ def user_group_update(operation_logger, groupname, add=None, remove=None, force=
     operation_logger.start()
 
     if set(new_group) != set(current_group):
+        ldap = _get_ldap_interface()
         if not ldap.update('cn=%s,ou=groups' % groupname, {"member": set(new_group_dns), "memberUid": set(new_group)}):
             raise YunohostError('group_update_failed', group=groupname)
 

From 6276485665977aae1e6407714988ca354fad5779 Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Wed, 11 Sep 2019 04:06:12 +0200
Subject: [PATCH 10/45] Simplify permission_list ... it really sounds like we
 don't need all these options

---
 data/actionsmap/yunohost.yml | 27 ++----------
 locales/en.json              |  1 -
 src/yunohost/backup.py       |  6 +--
 src/yunohost/permission.py   | 79 +++++++-----------------------------
 src/yunohost/user.py         |  6 +--
 5 files changed, 23 insertions(+), 96 deletions(-)

diff --git a/data/actionsmap/yunohost.yml b/data/actionsmap/yunohost.yml
index d4940c043..2bca684cd 100644
--- a/data/actionsmap/yunohost.yml
+++ b/data/actionsmap/yunohost.yml
@@ -274,33 +274,12 @@ user:
                                 pattern: *pattern_username
 
         permission:
-            subcategory_help: Manage user permission
+            subcategory_help: Manage permissions
             actions:
                 ### user_permission_list()
                 list:
-                    action_help: List access to user and group
-                    api: GET /users/permissions/<app>
-                    arguments:
-                        -a:
-                            full: --app
-                            help: Application to manage the permission
-                            nargs: "*"
-                            metavar: APP
-                        -p:
-                            full: --permission
-                            help: Name of permission (main by default)
-                            nargs: "*"
-                            metavar: PERMISSION
-                        -u:
-                            full: --username
-                            help: Username
-                            nargs: "*"
-                            metavar: USER
-                        -g:
-                            full: --group
-                            help: Group name
-                            nargs: "*"
-                            metavar: GROUP
+                    action_help: List permissions and corresponding accesses
+                    api: GET /users/permissions/<permission>
 
                 ### user_permission_add()
                 add:
diff --git a/locales/en.json b/locales/en.json
index d3abf4fd0..b02bf2238 100644
--- a/locales/en.json
+++ b/locales/en.json
@@ -438,7 +438,6 @@
     "permission_deleted": "Permission '{permission:s}' for app {app:s} deleted",
     "permission_deletion_failed": "Permission '{permission:s}' for app {app:s} deletion failed",
     "permission_not_found": "Permission '{permission:s}' not found for application {app:s}",
-    "permission_name_not_valid": "Permission name '{permission:s}' not valid",
     "permission_update_failed": "Permission update failed",
     "permission_generated": "The permission database has been updated",
     "permission_updated": "Permission '{permission:s}' for app {app:s} updated",
diff --git a/src/yunohost/backup.py b/src/yunohost/backup.py
index f96146ea0..fdbd8c62c 100644
--- a/src/yunohost/backup.py
+++ b/src/yunohost/backup.py
@@ -1256,10 +1256,8 @@ class RestoreManager():
 
         # Restore permission for the app which is installed
         for per in old_apps_permission:
-            try:
-                permission_name, app_name = per['cn'][0].split('.')
-            except:
-                logger.warning(m18n.n('permission_name_not_valid', permission=per['cn'][0]))
+            # FIXME : will come here later to fix this following previous commits ...
+            permission_name, app_name = per['cn'][0].split('.')
             if _is_installed(app_name):
                 if not ldap.add('cn=%s,ou=permission' % per['cn'][0], per):
                     raise YunohostError('apps_permission_restoration_failed', permission=permission_name, app=app_name)
diff --git a/src/yunohost/permission.py b/src/yunohost/permission.py
index 0b77a3e5c..fbb43e8b3 100644
--- a/src/yunohost/permission.py
+++ b/src/yunohost/permission.py
@@ -42,79 +42,30 @@ logger = getActionLogger('yunohost.user')
 #
 
 
-def user_permission_list(app=None, permission=None, username=None, group=None):
+def user_permission_list():
     """
-    List permission for specific application
-
-    Keyword argument:
-        app        -- an application OR sftp, xmpp (metronome), mail
-        permission -- name of the permission ("main" by default)
-        username   -- Username to get informations
-        group      -- Groupname to get informations
+    List permissions and corresponding accesses
 
     """
 
-    from yunohost.utils.ldap import _get_ldap_interface
+    from yunohost.utils.ldap import _get_ldap_interface, _ldap_path_extract
+
+    # Fetch all permissions objects
     ldap = _get_ldap_interface()
-
-    permission_attrs = [
-        'cn',
-        'groupPermission',
-        'inheritPermission',
-        'URL',
-    ]
-
-    # Normally app is alway defined but it should be possible to set it
-    if app and not isinstance(app, list):
-        app = [app]
-    if permission and not isinstance(permission, list):
-        permission = [permission]
-    if not isinstance(username, list):
-        username = [username]
-    if not isinstance(group, list):
-        group = [group]
+    permissions_infos = ldap.search('ou=permission,dc=yunohost,dc=org',
+                                    '(objectclass=permissionYnh)',
+                                    ['cn', 'groupPermission', 'inheritPermission', 'URL'])
 
     permissions = {}
+    for infos in permissions_infos:
 
-    result = ldap.search('ou=permission,dc=yunohost,dc=org',
-                         '(objectclass=permissionYnh)', permission_attrs)
+        name = infos['cn'][0]
 
-    for res in result:
-        try:
-            permission_name, app_name = res['cn'][0].split('.')
-        except:
-            logger.warning(m18n.n('permission_name_not_valid', permission=res['cn'][0]))
-        group_name = []
-        if 'groupPermission' in res:
-            for g in res['groupPermission']:
-                group_name.append(g.split("=")[1].split(",")[0])
-        user_name = []
-        if 'inheritPermission' in res:
-            for u in res['inheritPermission']:
-                user_name.append(u.split("=")[1].split(",")[0])
-
-        # Don't show the result if the user defined a specific permission, user or group
-        if app and app_name not in app:
-            continue
-        if permission and permission_name not in permission:
-            continue
-        if username[0] and not set(username) & set(user_name):
-            continue
-        if group[0] and not set(group) & set(group_name):
-            continue
-
-        if app_name not in permissions:
-            permissions[app_name] = {}
-
-        permissions[app_name][permission_name] = {'allowed_users': [], 'allowed_groups': []}
-        for g in group_name:
-            permissions[app_name][permission_name]['allowed_groups'].append(g)
-        for u in user_name:
-            permissions[app_name][permission_name]['allowed_users'].append(u)
-        if 'URL' in res:
-            permissions[app_name][permission_name]['URL'] = []
-            for u in res['URL']:
-                permissions[app_name][permission_name]['URL'].append(u)
+        permissions[name] = {
+            "allowed_users": [_ldap_path_extract(p, "uid") for p in infos.get('inheritPermission', [])],
+            "allowed_groups": [_ldap_path_extract(p, "cn") for p in infos.get('groupPermission', [])],
+            "urls": infos.get("URL", [])
+        }
 
     return {'permissions': permissions}
 
diff --git a/src/yunohost/user.py b/src/yunohost/user.py
index 8427cbd42..3eb329f4e 100644
--- a/src/yunohost/user.py
+++ b/src/yunohost/user.py
@@ -453,7 +453,7 @@ def user_info(username):
 
         if service_status("dovecot")["status"] != "running":
             logger.warning(m18n.n('mailbox_used_space_dovecot_down'))
-        elif not user_permission_list(app="mail", permission="main", username=username)['permissions']:
+        elif username not in user_permission_list()["permissions"]["mail.main"]["allowed_users"]:
             logger.warning(m18n.n('mailbox_disabled', user=username))
         else:
             cmd = 'doveadm -f flow quota get -u %s' % user['uid'][0]
@@ -719,9 +719,9 @@ def user_group_info(groupname):
 # Permission subcategory
 #
 
-def user_permission_list(app=None, permission=None, username=None, group=None, sync_perm=True):
+def user_permission_list():
     import yunohost.permission
-    return yunohost.permission.user_permission_list(app, permission, username, group)
+    return yunohost.permission.user_permission_list()
 
 
 @is_unit_operation([('app', 'user')])

From 41e6f1b81cdf032949f960fb7095a697d4af4256 Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Wed, 11 Sep 2019 14:37:15 +0200
Subject: [PATCH 11/45] Simplify permission_add/remove to just
 permission_update with --add and --remove, similar to what's done for groups

---
 data/actionsmap/yunohost.yml |  63 +++---------
 locales/en.json              |   6 +-
 src/yunohost/permission.py   | 180 ++++++++++++-----------------------
 src/yunohost/user.py         |  18 +---
 4 files changed, 84 insertions(+), 183 deletions(-)

diff --git a/data/actionsmap/yunohost.yml b/data/actionsmap/yunohost.yml
index 2bca684cd..ebdd2b982 100644
--- a/data/actionsmap/yunohost.yml
+++ b/data/actionsmap/yunohost.yml
@@ -276,64 +276,31 @@ user:
         permission:
             subcategory_help: Manage permissions
             actions:
+
                 ### user_permission_list()
                 list:
                     action_help: List permissions and corresponding accesses
                     api: GET /users/permissions/<permission>
 
-                ### user_permission_add()
-                add:
-                    action_help: Grant access right to users and group
-                    api: POST /users/permissions/<app>
+                ### user_permission_update()
+                update:
+                    action_help: Grant / remove permissions to groups or users
+                    api: POST /users/permissions/<permission>
                     arguments:
-                        app:
-                            help: Application to manage the permission
-                            nargs: "+"
-                        -p:
-                            full: --permission
-                            help: Name of permission (main by default)
+                        permission:
+                            help: Permission to manage (e.g. mail.main or wordpress.editors)
+                        -a:
+                            full: --add
+                            help: Group or user names to add to this permission
                             nargs: "*"
-                            metavar: PERMISSION
-                        -u:
-                            full: --username
-                            help: Username
-                            nargs: "*"
-                            metavar: USER
+                            metavar: GROUP_OR_USER
                             extra:
                                 pattern: *pattern_username
-                        -g:
-                            full: --group
-                            help: Group name
+                        -r:
+                            full: --remove
+                            help: Group or user names to remove from this permission
                             nargs: "*"
-                            metavar: GROUP
-                            extra:
-                                pattern: *pattern_username
-
-                ### user_permission_remove()
-                remove:
-                    action_help: Revoke access right to users and group
-                    api: PUT /users/permissions/<app>
-                    arguments:
-                        app:
-                            help: Application to manage the permission
-                            nargs: "+"
-                        -p:
-                            full: --permission
-                            help: Name of permission (main by default)
-                            nargs: "*"
-                            metavar: PERMISSION
-                        -u:
-                            full: --username
-                            help: Username
-                            nargs: "*"
-                            metavar: USER
-                            extra:
-                                pattern: *pattern_username
-                        -g:
-                            full: --group
-                            help: Group name
-                            nargs: "*"
-                            metavar: GROUP
+                            metavar: GROUP_OR_USER
                             extra:
                                 pattern: *pattern_username
 
diff --git a/locales/en.json b/locales/en.json
index b02bf2238..725bb1f8c 100644
--- a/locales/en.json
+++ b/locales/en.json
@@ -196,7 +196,6 @@
     "dyndns_registration_failed": "Unable to register DynDNS domain: {error:s}",
     "dyndns_domain_not_provided": "Dyndns provider {provider:s} cannot provide domain {domain:s}.",
     "dyndns_unavailable": "Domain {domain:s} is not available.",
-    "edit_permission_with_group_all_users_not_allowed": "You are not allowed to edit permission for group 'all_users', use 'yunohost user permission clear APP' or 'yunohost user permission add APP -u USER' instead.",
     "error_when_removing_sftpuser_group": "Error when trying remove sftpusers group",
     "executing_command": "Executing command '{command:s}'…",
     "executing_script": "Executing script '{script:s}'…",
@@ -229,8 +228,8 @@
     "global_settings_unknown_type": "Unexpected situation, the setting {setting:s} appears to have the type {unknown_type:s} but it's not a type supported by the system.",
     "good_practices_about_admin_password": "You are now about to define a new administration password. The password should be at least 8 characters - though it is good practice to use longer password (i.e. a passphrase) and/or to use various kind of characters (uppercase, lowercase, digits and special characters).",
     "good_practices_about_user_password": "You are now about to define a new user password. The password should be at least 8 characters - though it is good practice to use longer password (i.e. a passphrase) and/or to use various kind of characters (uppercase, lowercase, digits and special characters).",
-    "group_already_allowed": "Group '{group:s}' already has permission '{permission:s}' enabled for app '{app:s}'",
-    "group_already_disallowed": "Group '{group:s}' already has permissions '{permission:s}' disabled for app '{app:s}'",
+    "group_already_allowed": "Group '{group:s}' already has permission '{permission:s}' enabled'",
+    "group_already_disallowed": "Group '{group:s}' already has permission '{permission:s}' disabled'",
     "group_name_already_exist": "Group {name:s} already exist",
     "group_created": "Group '{group}' successfully created",
     "group_creation_failed": "Group creation failed for group '{group}'",
@@ -397,7 +396,6 @@
     "mysql_db_creation_failed": "MySQL database creation failed",
     "mysql_db_init_failed": "MySQL database init failed",
     "mysql_db_initialized": "The MySQL database has been initialized",
-    "need_define_permission_before": "You need to redefine the permission using 'yunohost user permission add -u USER' before removing an allowed group",
     "network_check_mx_ko": "DNS MX record is not set",
     "network_check_smtp_ko": "Outbound mail (SMTP port 25) seems to be blocked by your network",
     "network_check_smtp_ok": "Outbound mail (SMTP port 25) is not blocked",
diff --git a/src/yunohost/permission.py b/src/yunohost/permission.py
index fbb43e8b3..20c34ada8 100644
--- a/src/yunohost/permission.py
+++ b/src/yunohost/permission.py
@@ -24,6 +24,7 @@
     Manage permissions
 """
 
+import copy
 import grp
 import random
 
@@ -45,7 +46,6 @@ logger = getActionLogger('yunohost.user')
 def user_permission_list():
     """
     List permissions and corresponding accesses
-
     """
 
     from yunohost.utils.ldap import _get_ldap_interface, _ldap_path_extract
@@ -70,146 +70,92 @@ def user_permission_list():
     return {'permissions': permissions}
 
 
-def user_permission_update(operation_logger, app=[], permission=None, add_username=None, add_group=None, del_username=None, del_group=None, sync_perm=True):
+def user_permission_update(operation_logger, permission, add=None, remove=None, sync_perm=True):
     """
     Allow or Disallow a user or group to a permission for a specific application
 
     Keyword argument:
-        app            -- an application OR sftp, xmpp (metronome), mail
-        permission     -- name of the permission ("main" by default)
-        add_username   -- Username to allow
-        add_group      -- Groupname to allow
-        del_username   -- Username to disallow
-        del_group      -- Groupname to disallow
-
+        permission     -- Name of the permission (e.g. mail.mail or wordpress.editors)
+        add            -- List of groups or usernames to add to this permission
+        remove         -- List of groups or usernames to remove from to this permission
     """
     from yunohost.hook import hook_callback
     from yunohost.user import user_group_list
-    from yunohost.utils.ldap import _get_ldap_interface
+    from yunohost.utils.ldap import _get_ldap_interface, _ldap_path_extract
     ldap = _get_ldap_interface()
 
-    if permission:
-        if not isinstance(permission, list):
-            permission = [permission]
-    else:
-        permission = ["main"]
+    # Fetch currently allowed groups for this permission
 
-    if add_group:
-        if not isinstance(add_group, list):
-            add_group = [add_group]
-    else:
-        add_group = []
-
-    if add_username:
-        if not isinstance(add_username, list):
-            add_username = [add_username]
-    else:
-        add_username = []
-
-    if del_group:
-        if not isinstance(del_group, list):
-            del_group = [del_group]
-    else:
-        del_group = []
-
-    if del_username:
-        if not isinstance(del_username, list):
-            del_username = [del_username]
-    else:
-        del_username = []
-
-    # Validate that the group exist
-    for g in add_group:
-        if g not in user_group_list()['groups']:
-            raise YunohostError('group_unknown', group=g)
-    for u in add_username:
-        if u not in user_list(['uid'])['users']:
-            raise YunohostError('user_unknown', user=u)
-    for g in del_group:
-        if g not in user_group_list()['groups']:
-            raise YunohostError('group_unknown', group=g)
-    for u in del_username:
-        if u not in user_list(['uid'])['users']:
-            raise YunohostError('user_unknown', user=u)
-
-    # Merge user and group (note that we consider all user as a group)
-    add_group.extend(add_username)
-    del_group.extend(del_username)
-
-    if 'all_users' in add_group or 'all_users' in del_group:
-        raise YunohostError('edit_permission_with_group_all_users_not_allowed')
-
-    # Populate permission informations
-    permission_attrs = [
-        'cn',
-        'groupPermission',
-    ]
     result = ldap.search('ou=permission,dc=yunohost,dc=org',
-                         '(objectclass=permissionYnh)', permission_attrs)
+                         '(objectclass=permissionYnh)',
+                         ["cn", "groupPermission"])
     result = {p['cn'][0]: p for p in result}
+    if permission not in result:
+        raise YunohostError('permission_not_found', permission=permission)
 
-    new_per_dict = {}
+    current_allowed_groups = [_ldap_path_extract(p, "cn") for p in result[permission].get("groupPermission", [])]
 
-    for a in app:
-        for per in permission:
-            permission_name = per + '.' + a
-            if permission_name not in result:
-                raise YunohostError('permission_not_found', permission=per, app=a)
-            new_per_dict[permission_name] = set()
-            if 'groupPermission' in result[permission_name]:
-                new_per_dict[permission_name] = set(result[permission_name]['groupPermission'])
+    # Compute new allowed group list (and make sure what we're doing make sense)
 
-            for g in del_group:
-                if 'cn=all_users,ou=groups,dc=yunohost,dc=org' in new_per_dict[permission_name]:
-                    raise YunohostError('need_define_permission_before')
-                group_name = 'cn=' + g + ',ou=groups,dc=yunohost,dc=org'
-                if group_name not in new_per_dict[permission_name]:
-                    logger.warning(m18n.n('group_already_disallowed', permission=per, app=a, group=g))
-                else:
-                    new_per_dict[permission_name].remove(group_name)
+    new_allowed_groups = copy.copy(current_allowed_groups)
 
-            if 'cn=all_users,ou=groups,dc=yunohost,dc=org' in new_per_dict[permission_name]:
-                new_per_dict[permission_name].remove('cn=all_users,ou=groups,dc=yunohost,dc=org')
-            for g in add_group:
-                group_name = 'cn=' + g + ',ou=groups,dc=yunohost,dc=org'
-                if group_name in new_per_dict[permission_name]:
-                    logger.warning(m18n.n('group_already_allowed', permission=per, app=a, group=g))
-                else:
-                    new_per_dict[permission_name].add(group_name)
+    if add:
+        existing_groups = user_group_list()['groups'].keys()
+        groups_to_add = [add] if not isinstance(add, list) else add
+        for group in groups_to_add:
+            if group not in existing_groups:
+                raise YunohostError('group_unknown', group=group)
+            if group in current_allowed_groups:
+                logger.warning(m18n.n('group_already_allowed', permission=permission, group=group))
+        new_allowed_groups += groups_to_add
+
+    if remove:
+        groups_to_remove = [remove] if not isinstance(remove, list) else remove
+        for group in groups_to_remove:
+            if group not in existing_groups:
+                raise YunohostError('group_unknown', group=group)
+            if group not in current_allowed_groups:
+                logger.warning(m18n.n('group_already_disallowed', permission=permission, group=group))
+
+        new_allowed_groups = [g for g in new_allowed_groups if g not in groups_to_remove]
+
+    # If we end up with something like allowed groups is ["all_users", "volunteers"]
+    # we shall warn the users that they should probably choose between one or the other,
+    # because the current situation is probably not what they expect / is temporary ?
+
+    if len(new_allowed_groups) > 1 and "all_users" in new_allowed_groups:
+        # FIXME : write a better explanation
+        logger.warning("This permission is currently enabled for all users in addition to other groups. You probably want to either remove the 'all_users' permission or remove the specific groups currently allowed.")
+
+    # Commit the new allowed group list
 
     operation_logger.start()
 
-    for per, val in new_per_dict.items():
-        # Don't update LDAP if we update exactly the same values
-        if val == set(result[per]['groupPermission'] if 'groupPermission' in result[per] else []):
-            continue
-        if ldap.update('cn=%s,ou=permission' % per, {'groupPermission': val}):
-            p = per.split('.')
-            logger.debug(m18n.n('permission_updated', permission=p[0], app=p[1]))
-        else:
-            raise YunohostError('permission_update_failed')
+    # Don't update LDAP if we update exactly the same values
+    if set(new_allowed_groups) == set(current_allowed_groups):
+        logger.warning("No change was applied because not relevant modification were found")
+    elif ldap.update('cn=%s,ou=permission' % permission,
+                     {'groupPermission': ['cn=' + g + ',ou=groups,dc=yunohost,dc=org' for g in new_allowed_groups]}):
+        logger.debug(m18n.n('permission_updated', permission=permission))
 
-    if sync_perm:
-        permission_sync_to_user()
+        # Trigger permission sync if asked
 
-    for a in app:
-        allowed_users = set()
-        disallowed_users = set()
-        group_list = user_group_list()['groups']
+        if sync_perm:
+            permission_sync_to_user()
 
-        for g in add_group:
-            allowed_users.union(group_list[g]['members'])
-        for g in del_group:
-            disallowed_users.union(group_list[g]['members'])
+        # Trigger app callbacks
 
-        allowed_users = ','.join(allowed_users)
-        disallowed_users = ','.join(disallowed_users)
-        if add_group:
-            hook_callback('post_app_addaccess', args=[app, allowed_users])
-        if del_group:
-            hook_callback('post_app_removeaccess', args=[app, disallowed_users])
+        # FIXME FIXME FIXME
 
-    return user_permission_list(app, permission)
+        #if groups_to_add:
+        #    hook_callback('post_app_addaccess', args=[app, allowed_users])
+        #if groups_to_remove:
+        #    hook_callback('post_app_removeaccess', args=[app, disallowed_users])
+
+    else:
+        raise YunohostError('permission_update_failed')
+
+    return user_permission_list()["permissions"][permission]
 
 
 def user_permission_clear(operation_logger, app=[], permission=None, sync_perm=True):
diff --git a/src/yunohost/user.py b/src/yunohost/user.py
index 3eb329f4e..92bdcf7a4 100644
--- a/src/yunohost/user.py
+++ b/src/yunohost/user.py
@@ -724,21 +724,11 @@ def user_permission_list():
     return yunohost.permission.user_permission_list()
 
 
-@is_unit_operation([('app', 'user')])
-def user_permission_add(operation_logger, app, permission="main", username=None, group=None, sync_perm=True):
+@is_unit_operation([('permission', 'user')])
+def user_permission_update(operation_logger, permission, add=None, remove=None, sync_perm=True):
     import yunohost.permission
-    return yunohost.permission.user_permission_update(operation_logger, app, permission=permission,
-                                                      add_username=username, add_group=group,
-                                                      del_username=None, del_group=None,
-                                                      sync_perm=sync_perm)
-
-
-@is_unit_operation([('app', 'user')])
-def user_permission_remove(operation_logger, app, permission="main", username=None, group=None, sync_perm=True):
-    import yunohost.permission
-    return yunohost.permission.user_permission_update(operation_logger, app, permission=permission,
-                                                      add_username=None, add_group=None,
-                                                      del_username=username, del_group=group,
+    return yunohost.permission.user_permission_update(operation_logger, permission,
+                                                      add=add, remove=remove,
                                                       sync_perm=sync_perm)
 
 

From 45483f4116e487be9d344fadff222007087a28e7 Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Wed, 11 Sep 2019 15:16:09 +0200
Subject: [PATCH 12/45] --short and --full options for group_list and
 permission_list

---
 data/actionsmap/yunohost.yml | 18 ++++++++++++++----
 src/yunohost/permission.py   | 24 +++++++++++++++---------
 src/yunohost/user.py         | 28 +++++++++++-----------------
 3 files changed, 40 insertions(+), 30 deletions(-)

diff --git a/data/actionsmap/yunohost.yml b/data/actionsmap/yunohost.yml
index ebdd2b982..ece642d0d 100644
--- a/data/actionsmap/yunohost.yml
+++ b/data/actionsmap/yunohost.yml
@@ -208,13 +208,13 @@ user:
                     action_help: List existing groups
                     api: GET /users/groups
                     arguments:
-                        -n:
-                            full: --names-only
-                            help: Only list the name of the groups without any additional info
+                        -s:
+                            full: --short
+                            help: List only the names of groups
                             action: store_true
                         -f:
                             full: --full
-                            help: List all the info available for each groups
+                            help: Display all informations known about each groups
                             action: store_true
 
                 ### user_group_create()
@@ -281,6 +281,16 @@ user:
                 list:
                     action_help: List permissions and corresponding accesses
                     api: GET /users/permissions/<permission>
+                    arguments:
+                        -s:
+                            full: --short
+                            help: List only the names of permissions
+                            action: store_true
+                        -f:
+                            full: --full
+                            help: Display all informations known about each permissions, including the full list of users corresponding to allowed groups.
+                            action: store_true
+
 
                 ### user_permission_update()
                 update:
diff --git a/src/yunohost/permission.py b/src/yunohost/permission.py
index 20c34ada8..ab79ff7ed 100644
--- a/src/yunohost/permission.py
+++ b/src/yunohost/permission.py
@@ -43,29 +43,35 @@ logger = getActionLogger('yunohost.user')
 #
 
 
-def user_permission_list():
+def user_permission_list(short=False, full=False):
     """
     List permissions and corresponding accesses
     """
 
-    from yunohost.utils.ldap import _get_ldap_interface, _ldap_path_extract
+    # Fetch relevant informations
 
-    # Fetch all permissions objects
+    from yunohost.utils.ldap import _get_ldap_interface, _ldap_path_extract
     ldap = _get_ldap_interface()
     permissions_infos = ldap.search('ou=permission,dc=yunohost,dc=org',
                                     '(objectclass=permissionYnh)',
-                                    ['cn', 'groupPermission', 'inheritPermission', 'URL'])
+                                    ["cn", 'groupPermission', 'inheritPermission', 'URL'])
+
+    # Parse / organize information to be outputed
 
     permissions = {}
     for infos in permissions_infos:
 
         name = infos['cn'][0]
+        permissions[name] = {}
 
-        permissions[name] = {
-            "allowed_users": [_ldap_path_extract(p, "uid") for p in infos.get('inheritPermission', [])],
-            "allowed_groups": [_ldap_path_extract(p, "cn") for p in infos.get('groupPermission', [])],
-            "urls": infos.get("URL", [])
-        }
+        permissions[name]["allowed"] = [_ldap_path_extract(p, "cn") for p in infos.get('groupPermission', [])]
+
+        if full:
+            permissions[name]["corresponding_users"] = [_ldap_path_extract(p, "uid") for p in infos.get('inheritPermission', [])],
+            permissions[name]["urls"] = infos.get("URL", [])
+
+    if short:
+        permissions = permissions.keys()
 
     return {'permissions': permissions}
 
diff --git a/src/yunohost/user.py b/src/yunohost/user.py
index 92bdcf7a4..80f558809 100644
--- a/src/yunohost/user.py
+++ b/src/yunohost/user.py
@@ -489,12 +489,12 @@ def user_info(username):
 #
 # Group subcategory
 #
-def user_group_list(names_only=False, full=False):
+def user_group_list(short=False, full=False):
     """
     List users
 
     Keyword argument:
-        names-only -- Only list the name of the groups without any additional info
+        short -- Only list the name of the groups without any additional info
         full -- List all the info available for each groups
     """
 
@@ -502,30 +502,24 @@ def user_group_list(names_only=False, full=False):
 
     from yunohost.utils.ldap import _get_ldap_interface, _ldap_path_extract
     ldap = _get_ldap_interface()
-
-    if names_only:
-        fields_to_fetch = ["cn"]
-    elif full:
-        fields_to_fetch = ["cn", "member", "permission"]
-    else:
-        fields_to_fetch = ["cn", "member"]
-
     groups_infos = ldap.search('ou=groups,dc=yunohost,dc=org',
                                '(objectclass=groupOfNamesYnh)',
-                               fields_to_fetch)
+                               ["cn", "member", "permission"])
 
     # Parse / organize information to be outputed
 
     groups = {}
     for infos in groups_infos:
+
         name = infos["cn"][0]
         groups[name] = {}
-        if "member" in fields_to_fetch:
-            groups[name]["members"] = [_ldap_path_extract(p, "uid") for p in infos.get("member", [])]
-        if "permission" in fields_to_fetch:
+
+        groups[name]["members"] = [_ldap_path_extract(p, "uid") for p in infos.get("member", [])]
+
+        if full:
             groups[name]["permissions"] = [_ldap_path_extract(p, "cn") for p in infos.get("permission", [])]
 
-    if names_only:
+    if short:
         groups = groups.keys()
 
     return {'groups': groups}
@@ -719,9 +713,9 @@ def user_group_info(groupname):
 # Permission subcategory
 #
 
-def user_permission_list():
+def user_permission_list(short=False, full=False):
     import yunohost.permission
-    return yunohost.permission.user_permission_list()
+    return yunohost.permission.user_permission_list(short, full)
 
 
 @is_unit_operation([('permission', 'user')])

From e5676c4b30db38f63ec740ac987aa697f473173a Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Wed, 11 Sep 2019 15:16:26 +0200
Subject: [PATCH 13/45] Propagate change in permission_list to
 permission_update

---
 src/yunohost/permission.py | 36 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/src/yunohost/permission.py b/src/yunohost/permission.py
index ab79ff7ed..3a6bde077 100644
--- a/src/yunohost/permission.py
+++ b/src/yunohost/permission.py
@@ -92,33 +92,31 @@ def user_permission_update(operation_logger, permission, add=None, remove=None,
 
     # Fetch currently allowed groups for this permission
 
-    result = ldap.search('ou=permission,dc=yunohost,dc=org',
-                         '(objectclass=permissionYnh)',
-                         ["cn", "groupPermission"])
-    result = {p['cn'][0]: p for p in result}
-    if permission not in result:
+    permissions = user_permission_list(full=True)["permissions"]
+    if permission not in permissions:
         raise YunohostError('permission_not_found', permission=permission)
 
-    current_allowed_groups = [_ldap_path_extract(p, "cn") for p in result[permission].get("groupPermission", [])]
+    current_allowed_groups = permissions[permission]["allowed"]
+    all_existing_groups = user_group_list()['groups'].keys()
 
     # Compute new allowed group list (and make sure what we're doing make sense)
 
     new_allowed_groups = copy.copy(current_allowed_groups)
 
     if add:
-        existing_groups = user_group_list()['groups'].keys()
         groups_to_add = [add] if not isinstance(add, list) else add
         for group in groups_to_add:
-            if group not in existing_groups:
+            if group not in all_existing_groups:
                 raise YunohostError('group_unknown', group=group)
             if group in current_allowed_groups:
                 logger.warning(m18n.n('group_already_allowed', permission=permission, group=group))
+
         new_allowed_groups += groups_to_add
 
     if remove:
         groups_to_remove = [remove] if not isinstance(remove, list) else remove
         for group in groups_to_remove:
-            if group not in existing_groups:
+            if group not in all_existing_groups:
                 raise YunohostError('group_unknown', group=group)
             if group not in current_allowed_groups:
                 logger.warning(m18n.n('group_already_disallowed', permission=permission, group=group))
@@ -130,7 +128,8 @@ def user_permission_update(operation_logger, permission, add=None, remove=None,
     # because the current situation is probably not what they expect / is temporary ?
 
     if len(new_allowed_groups) > 1 and "all_users" in new_allowed_groups:
-        # FIXME : write a better explanation
+        # FIXME : i18n
+        # FIXME : write a better explanation ?
         logger.warning("This permission is currently enabled for all users in addition to other groups. You probably want to either remove the 'all_users' permission or remove the specific groups currently allowed.")
 
     # Commit the new allowed group list
@@ -139,6 +138,7 @@ def user_permission_update(operation_logger, permission, add=None, remove=None,
 
     # Don't update LDAP if we update exactly the same values
     if set(new_allowed_groups) == set(current_allowed_groups):
+        # FIXME : i18n
         logger.warning("No change was applied because not relevant modification were found")
     elif ldap.update('cn=%s,ou=permission' % permission,
                      {'groupPermission': ['cn=' + g + ',ou=groups,dc=yunohost,dc=org' for g in new_allowed_groups]}):
@@ -149,20 +149,20 @@ def user_permission_update(operation_logger, permission, add=None, remove=None,
         if sync_perm:
             permission_sync_to_user()
 
+        new_permission = user_permission_list(full=True)["permissions"][permission]
+
         # Trigger app callbacks
+        app = permission.split(".")[0]
+        if add:
+            hook_callback('post_app_addaccess', args=[app, new_permission["corresponding_users"]])
+        if remove:
+            hook_callback('post_app_removeaccess', args=[app, new_permission["corresponding_users"]])
 
-        # FIXME FIXME FIXME
-
-        #if groups_to_add:
-        #    hook_callback('post_app_addaccess', args=[app, allowed_users])
-        #if groups_to_remove:
-        #    hook_callback('post_app_removeaccess', args=[app, disallowed_users])
+        return new_permission
 
     else:
         raise YunohostError('permission_update_failed')
 
-    return user_permission_list()["permissions"][permission]
-
 
 def user_permission_clear(operation_logger, app=[], permission=None, sync_perm=True):
     """

From a1d3376613993a8420373eec1b05bddc08a7720f Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Wed, 11 Sep 2019 15:49:07 +0200
Subject: [PATCH 14/45] Simplify permission_clear, now named permission_reset

---
 data/actionsmap/yunohost.yml | 16 +++-----
 locales/en.json              |  1 -
 src/yunohost/permission.py   | 75 +++++++++++++-----------------------
 src/yunohost/user.py         |  4 +-
 4 files changed, 34 insertions(+), 62 deletions(-)

diff --git a/data/actionsmap/yunohost.yml b/data/actionsmap/yunohost.yml
index ece642d0d..49dde373b 100644
--- a/data/actionsmap/yunohost.yml
+++ b/data/actionsmap/yunohost.yml
@@ -314,19 +314,13 @@ user:
                             extra:
                                 pattern: *pattern_username
 
-                ## user_permission_clear()
-                clear:
-                    action_help: Reset access rights for the app
+                ## user_permission_reset()
+                reset:
+                    action_help: Reset allowed groups to the default (all_users) for a given permission
                     api: DELETE /users/permissions/<app>
                     arguments:
-                        app:
-                            help: Application to manage the permission
-                            nargs: "+"
-                        -p:
-                            full: --permission
-                            help: Name of permission (main by default)
-                            nargs: "*"
-                            metavar: PERMISSION
+                        permission:
+                            help: Permission to be resetted (e.g. mail.main or wordpress.editors)
 
         ssh:
             subcategory_help: Manage ssh access
diff --git a/locales/en.json b/locales/en.json
index 725bb1f8c..ebbb89fa8 100644
--- a/locales/en.json
+++ b/locales/en.json
@@ -429,7 +429,6 @@
     "pattern_positive_number": "Must be a positive number",
     "pattern_username": "Must be lower-case alphanumeric and underscore characters only",
     "pattern_password_app": "Sorry, passwords should not contain the following characters: {forbidden_chars}",
-    "permission_already_clear": "Permission '{permission:s}' already clear for app {app:s}",
     "permission_already_exist": "Permission '{permission:s}' for app {app:s} already exist",
     "permission_created": "Permission '{permission:s}' for app {app:s} created",
     "permission_creation_failed": "Permission creation failed",
diff --git a/src/yunohost/permission.py b/src/yunohost/permission.py
index 3a6bde077..8816ad950 100644
--- a/src/yunohost/permission.py
+++ b/src/yunohost/permission.py
@@ -152,11 +152,13 @@ def user_permission_update(operation_logger, permission, add=None, remove=None,
         new_permission = user_permission_list(full=True)["permissions"][permission]
 
         # Trigger app callbacks
-        app = permission.split(".")[0]
-        if add:
-            hook_callback('post_app_addaccess', args=[app, new_permission["corresponding_users"]])
-        if remove:
-            hook_callback('post_app_removeaccess', args=[app, new_permission["corresponding_users"]])
+        # FIXME : this is not how this hook works... gotta compute the list of user actually added / removed
+
+        #app = permission.split(".")[0]
+        #if add:
+        #    hook_callback('post_app_addaccess', args=[app, new_permission["corresponding_users"]])
+        #if remove:
+        #    hook_callback('post_app_removeaccess', args=[app, new_permission["corresponding_users"]])
 
         return new_permission
 
@@ -164,63 +166,40 @@ def user_permission_update(operation_logger, permission, add=None, remove=None,
         raise YunohostError('permission_update_failed')
 
 
-def user_permission_clear(operation_logger, app=[], permission=None, sync_perm=True):
+def user_permission_reset(operation_logger, permission, sync_perm=True):
     """
-    Reset the permission for a specific application
+    Reset a given permission to just 'all_users'
 
     Keyword argument:
-        app        -- an application OR sftp, xmpp (metronome), mail
-        permission -- name of the permission ("main" by default)
-        username   -- Username to get informations (all by default)
-        group      -- Groupname to get informations (all by default)
-
+        permission -- The name of the permission to be reseted
     """
     from yunohost.hook import hook_callback
     from yunohost.utils.ldap import _get_ldap_interface
     ldap = _get_ldap_interface()
 
-    if permission:
-        if not isinstance(permission, list):
-            permission = [permission]
-    else:
-        permission = ["main"]
+    # Fetch existing permission
+
+    existing_permission = user_permission_list(full=True)["permissions"].get(permission, None)
+    if existing_permission is None:
+        raise YunohostError('permission_not_found', permission=permission)
+
+    # Update permission with default (all_users)
 
     default_permission = {'groupPermission': ['cn=all_users,ou=groups,dc=yunohost,dc=org']}
+    if ldap.update('cn=%s,ou=permission' % permission, default_permission):
+        logger.debug(m18n.n('permission_updated', permission=permission))
+    else:
+        raise YunohostError('permission_update_failed')
 
-    # Populate permission informations
-    permission_attrs = [
-        'cn',
-        'groupPermission',
-    ]
-    result = ldap.search('ou=permission,dc=yunohost,dc=org',
-                         '(objectclass=permissionYnh)', permission_attrs)
-    result = {p['cn'][0]: p for p in result}
+    if sync_perm:
+        permission_sync_to_user()
 
-    for a in app:
-        for per in permission:
-            permission_name = per + '.' + a
-            if permission_name not in result:
-                raise YunohostError('permission_not_found', permission=per, app=a)
-            if 'groupPermission' in result[permission_name] and 'cn=all_users,ou=groups,dc=yunohost,dc=org' in result[permission_name]['groupPermission']:
-                logger.warning(m18n.n('permission_already_clear', permission=per, app=a))
-                continue
-            if ldap.update('cn=%s,ou=permission' % permission_name, default_permission):
-                logger.debug(m18n.n('permission_updated', permission=per, app=a))
-            else:
-                raise YunohostError('permission_update_failed')
+    new_permission = user_permission_list(full=True)["permissions"][permission]
 
-    permission_sync_to_user()
+    # FIXME : trigger app callbacks
+    # app = permission.split(".")[0]
 
-    for a in app:
-        permission_name = 'main.' + a
-        result = ldap.search('ou=permission,dc=yunohost,dc=org',
-                             filter='cn=' + permission_name, attrs=['inheritPermission'])
-        if result:
-            allowed_users = result[0]['inheritPermission']
-            new_user_list = ','.join(allowed_users)
-            hook_callback('post_app_removeaccess', args=[app, new_user_list])
-
-    return user_permission_list(app, permission)
+    return new_permission
 
 #
 #
diff --git a/src/yunohost/user.py b/src/yunohost/user.py
index 80f558809..2bf36cfd6 100644
--- a/src/yunohost/user.py
+++ b/src/yunohost/user.py
@@ -727,9 +727,9 @@ def user_permission_update(operation_logger, permission, add=None, remove=None,
 
 
 @is_unit_operation([('app', 'user')])
-def user_permission_clear(operation_logger, app, permission=None, sync_perm=True):
+def user_permission_reset(operation_logger, permission, sync_perm=True):
     import yunohost.permission
-    return yunohost.permission.user_permission_clear(operation_logger, app, permission,
+    return yunohost.permission.user_permission_reset(operation_logger, permission,
                                                      sync_perm=sync_perm)
 
 

From 3535cb655f17be09ede5157473b28cdc09060b2b Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Wed, 11 Sep 2019 16:36:17 +0200
Subject: [PATCH 15/45] Fix call of app add/remove access hooks

---
 src/yunohost/permission.py | 42 +++++++++++++++++++++++++++-----------
 1 file changed, 30 insertions(+), 12 deletions(-)

diff --git a/src/yunohost/permission.py b/src/yunohost/permission.py
index 8816ad950..fbfaa43a2 100644
--- a/src/yunohost/permission.py
+++ b/src/yunohost/permission.py
@@ -67,7 +67,7 @@ def user_permission_list(short=False, full=False):
         permissions[name]["allowed"] = [_ldap_path_extract(p, "cn") for p in infos.get('groupPermission', [])]
 
         if full:
-            permissions[name]["corresponding_users"] = [_ldap_path_extract(p, "uid") for p in infos.get('inheritPermission', [])],
+            permissions[name]["corresponding_users"] = [_ldap_path_extract(p, "uid") for p in infos.get('inheritPermission', [])]
             permissions[name]["urls"] = infos.get("URL", [])
 
     if short:
@@ -92,11 +92,11 @@ def user_permission_update(operation_logger, permission, add=None, remove=None,
 
     # Fetch currently allowed groups for this permission
 
-    permissions = user_permission_list(full=True)["permissions"]
-    if permission not in permissions:
+    existing_permission = user_permission_list(full=True)["permissions"].get(permission, None)
+    if existing_permission is None:
         raise YunohostError('permission_not_found', permission=permission)
 
-    current_allowed_groups = permissions[permission]["allowed"]
+    current_allowed_groups = existing_permission["allowed"]
     all_existing_groups = user_group_list()['groups'].keys()
 
     # Compute new allowed group list (and make sure what we're doing make sense)
@@ -152,13 +152,19 @@ def user_permission_update(operation_logger, permission, add=None, remove=None,
         new_permission = user_permission_list(full=True)["permissions"][permission]
 
         # Trigger app callbacks
-        # FIXME : this is not how this hook works... gotta compute the list of user actually added / removed
 
-        #app = permission.split(".")[0]
-        #if add:
-        #    hook_callback('post_app_addaccess', args=[app, new_permission["corresponding_users"]])
-        #if remove:
-        #    hook_callback('post_app_removeaccess', args=[app, new_permission["corresponding_users"]])
+        app = permission.split(".")[0]
+
+        old_allowed_users = set(existing_permission["corresponding_users"])
+        new_allowed_users = set(new_permission["corresponding_users"])
+
+        effectively_added_users = new_allowed_users - old_allowed_users
+        effectively_removed_users = old_allowed_users - new_allowed_users
+
+        if effectively_added_users:
+            hook_callback('post_app_addaccess', args=[app, ','.join(effectively_added_users)])
+        if effectively_removed_users:
+            hook_callback('post_app_removeaccess', args=[app, ','.join(effectively_removed_users)])
 
         return new_permission
 
@@ -196,8 +202,20 @@ def user_permission_reset(operation_logger, permission, sync_perm=True):
 
     new_permission = user_permission_list(full=True)["permissions"][permission]
 
-    # FIXME : trigger app callbacks
-    # app = permission.split(".")[0]
+    # Trigger app callbacks
+
+    app = permission.split(".")[0]
+
+    old_allowed_users = set(existing_permission["corresponding_users"])
+    new_allowed_users = set(new_permission["corresponding_users"])
+
+    effectively_added_users = new_allowed_users - old_allowed_users
+    effectively_removed_users = old_allowed_users - new_allowed_users
+
+    if effectively_added_users:
+        hook_callback('post_app_addaccess', args=[app, ','.join(effectively_added_users)])
+    if effectively_removed_users:
+        hook_callback('post_app_removeaccess', args=[app, ','.join(effectively_removed_users)])
 
     return new_permission
 

From 574e9aea44a8e25078b506240bc7cedf155dd8e0 Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Wed, 11 Sep 2019 18:13:30 +0200
Subject: [PATCH 16/45] Simplify permission_create/urls/delete interface and
 code

---
 data/helpers.d/setting     |  57 +++++++--------
 src/yunohost/permission.py | 138 ++++++++++++++++++-------------------
 2 files changed, 93 insertions(+), 102 deletions(-)

diff --git a/data/helpers.d/setting b/data/helpers.d/setting
index e6311fc1f..0e432d916 100644
--- a/data/helpers.d/setting
+++ b/data/helpers.d/setting
@@ -230,70 +230,63 @@ ynh_webpath_register () {
 
 # Create a new permission for the app
 #
-# usage: ynh_permission_create --app "app" --permission "permission" --defaultdisallow [--urls "url" ["url" ...]]
-# | arg: app - the application id
+# usage: ynh_permission_create --permission "permission" [--urls "url" ["url" ...]]
 # | arg: permission - the name for the permission (by default a permission named "main" already exist)
-# | arg: defaultdisallow - define if all user will be allowed by default
-# | arg: urls - the list of urls for the the permission
+# | arg: urls - (optional) a list of FULL urls for the permission (e.g. domain.tld/apps/admin)
+#
+# example: ynh_permission_create --permission admin --urls domain.tld/blog/admin
 ynh_permission_create() {
-    declare -Ar args_array=( [a]=app= [p]=permission= [d]=defaultdisallow [u]=urls= )
-    local app
+    declare -Ar args_array=( [p]=permission= [u]=urls= )
     local permission
-    local defaultdisallow
     local urls
     ynh_handle_getopts_args "$@"
-    if [[ -n ${defaultdisallow:-} ]]; then
-        defaultdisallow=",default_allow=False"
-    fi
 
     if [[ -n ${urls:-} ]]; then
         urls=",urls=['${urls//';'/"','"}']"
     fi
-    yunohost tools shell -c "from yunohost.permission import permission_create; permission_create('$app', '$permission' ${defaultdisallow:-} ${urls:-}, sync_perm=False)"
+    yunohost tools shell -c "from yunohost.permission import permission_create; permission_create('$app.$permission' ${urls:-}, sync_perm=False)"
 }
 
 # Remove a permission for the app (note that when the app is removed all permission is automatically removed)
 #
-# usage: ynh_permission_remove --app "app" --permission "permission"
-# | arg: app - the application id
+# usage: ynh_permission_remove --permission "permission"
 # | arg: permission - the name for the permission (by default a permission named "main" is removed automatically when the app is removed)
-ynh_permission_remove() {
-    declare -Ar args_array=( [a]=app= [p]=permission= )
-    local app
+#
+# example: ynh_permission_delete --permission editors
+ynh_permission_delete() {
+    declare -Ar args_array=( [p]=permission= )
     local permission
     ynh_handle_getopts_args "$@"
 
-    yunohost tools shell -c "from yunohost.permission import permission_delete; permission_delete('$app', '$permission', sync_perm=False)"
+    yunohost tools shell -c "from yunohost.permission import permission_delete; permission_delete('$app.$permission', sync_perm=False)"
 }
 
 # Add a path managed by the SSO
 #
-# usage: ynh_permission_add_url --app "app" --permission "permission" --url "url" ["url" ...]
-# | arg: app - the application id
-# | arg: permission - the name for the permission
-# | arg: url - the FULL url for the the permission (ex domain.tld/apps/admin)
+# usage: ynh_permission_add_url --permission "permission" --url "url" ["url" ...]
+# | arg: permission - the name for the permission (by default a permission named "main" is removed automatically when the app is removed)
+# | arg: urls - (optional) a list of FULL urls for the permission (e.g. domain.tld/apps/admin)
+#
 ynh_permission_add_url() {
-    declare -Ar args_array=( [a]=app= [p]=permission= [u]=url= )
-    local app
+    declare -Ar args_array=([p]=permission= [u]=urls= )
     local permission
-    local url
+    local urls
     ynh_handle_getopts_args "$@"
 
-    yunohost tools shell -c "from yunohost.permission import permission_urls; permission_urls('$app', '$permission', add_url=['${url//';'/"','"}'], sync_perm=False)"
+    yunohost tools shell -c "from yunohost.permission import permission_urls; permission_urls('$app.$permission', add=['${urls//';'/"','"}'], sync_perm=False)"
 }
 
 # Remove a path managed by the SSO
 #
 # usage: ynh_permission_del_path --app "app" --permission "permission" --url "url" ["url" ...]
-# | arg: app - the application id
-# | arg: permission - the name for the permission
-# | arg: url - the FULL url for the the permission (ex domain.tld/apps/admin)
+# | arg: permission - the name for the permission (by default a permission named "main" is removed automatically when the app is removed)
+# | arg: urls - (optional) a list of FULL urls for the permission (e.g. domain.tld/apps/admin)
+#
 ynh_permission_remove_url() {
-    declare -Ar args_array=( [a]=app= [p]=permission= [u]=url= )
-    local app
+    declare -Ar args_array=([p]=permission= [u]=urls= )
     local permission
-    local url
+    local urls
     ynh_handle_getopts_args "$@"
 
-    yunohost tools shell -c "from yunohost.permission import permission_urls; permission_urls('$app', '$permission', remove_url=['${url//';'/"','"}'], sync_perm=False)"
+    yunohost tools shell -c "from yunohost.permission import permission_urls; permission_urls('$app.$permission', remove=['${url//';'/"','"}'], sync_perm=False)"
 }
diff --git a/src/yunohost/permission.py b/src/yunohost/permission.py
index fbfaa43a2..960c0853c 100644
--- a/src/yunohost/permission.py
+++ b/src/yunohost/permission.py
@@ -229,27 +229,22 @@ def user_permission_reset(operation_logger, permission, sync_perm=True):
 
 
 @is_unit_operation(['permission', 'app'])
-def permission_create(operation_logger, app, permission, urls=None, default_allow=True, sync_perm=True):
+def permission_create(operation_logger, permission, urls=None, sync_perm=True):
     """
     Create a new permission for a specific application
 
     Keyword argument:
-        app        -- an application OR sftp, xmpp (metronome), mail
-        permission -- name of the permission ("main" by default)
+        permission -- Name of the permission (e.g. nextcloud.main or wordpress.editors)
         urls       -- list of urls to specify for the permission
-
     """
-    from yunohost.domain import _normalize_domain_path
+
     from yunohost.utils.ldap import _get_ldap_interface
     ldap = _get_ldap_interface()
 
     # Validate uniqueness of permission in LDAP
-    permission_name = str(permission + '.' + app)  # str(...) Fix encoding issue
-    conflict = ldap.get_conflict({
-        'cn': permission_name
-    }, base_dn='ou=permission,dc=yunohost,dc=org')
-    if conflict:
-        raise YunohostError('permission_already_exist', permission=permission, app=app)
+    if ldap.get_conflict({'cn': permission},
+                         base_dn='ou=permission,dc=yunohost,dc=org'):
+        raise YunohostError('permission_already_exist', permission=permission)
 
     # Get random GID
     all_gid = {x.gr_gid for x in grp.getgrall()}
@@ -261,110 +256,106 @@ def permission_create(operation_logger, app, permission, urls=None, default_allo
 
     attr_dict = {
         'objectClass': ['top', 'permissionYnh', 'posixGroup'],
-        'cn': permission_name,
+        'cn': permission,
         'gidNumber': gid,
     }
-    if default_allow:
+
+    # For main permission, we add all users by default
+    if permission.endswith(".main"):
         attr_dict['groupPermission'] = 'cn=all_users,ou=groups,dc=yunohost,dc=org'
 
     if urls:
-        attr_dict['URL'] = []
-        for url in urls:
-            domain = url[:url.index('/')]
-            path = url[url.index('/'):]
-            domain, path = _normalize_domain_path(domain, path)
-            attr_dict['URL'].append(domain + path)
+        attr_dict['URL'] = [_normalize_url(url) for url in urls]
 
     operation_logger.start()
-    if ldap.add('cn=%s,ou=permission' % permission_name, attr_dict):
+    if ldap.add('cn=%s,ou=permission' % permission, attr_dict):
         if sync_perm:
             permission_sync_to_user()
-        logger.debug(m18n.n('permission_created', permission=permission, app=app))
-        return user_permission_list(app, permission)
-
-    raise YunohostError('permission_creation_failed')
+        logger.debug(m18n.n('permission_created', permission=permission))
+        return user_permission_list(full=True)["permissions"][permission]
+    else:
+        raise YunohostError('permission_creation_failed')
 
 
 @is_unit_operation(['permission', 'app'])
-def permission_urls(operation_logger, app, permission, add_url=None, remove_url=None, sync_perm=True):
+def permission_urls(operation_logger, permission, add=None, remove=None, sync_perm=True):
     """
     Update urls related to a permission for a specific application
 
     Keyword argument:
-        app            -- an application OR sftp, xmpp (metronome), mail
-        permission     -- name of the permission ("main" by default)
-        add_url        -- Add a new url for a permission
-        remove_url     -- Remove a url for a permission
+        permission -- Name of the permission (e.g. nextcloud.main or wordpress.editors)
+        add        -- List of urls to add
+        remove     -- List of urls to remove
 
     """
-    from yunohost.domain import _normalize_domain_path
     from yunohost.utils.ldap import _get_ldap_interface
     ldap = _get_ldap_interface()
 
-    permission_name = str(permission + '.' + app)  # str(...) Fix encoding issue
+    # Fetch existing permission
 
-    # Populate permission informations
-    result = ldap.search(base='ou=permission,dc=yunohost,dc=org',
-                         filter='cn=' + permission_name, attrs=['URL'])
-    if not result:
-        raise YunohostError('permission_not_found', permission=permission, app=app)
-    permission_obj = result[0]
+    existing_permission = user_permission_list(full=True)["permissions"].get(permission, None)
+    if not existing_permission:
+        raise YunohostError('permission_not_found', permission=permission)
 
-    if 'URL' not in permission_obj:
-        permission_obj['URL'] = []
+    # Compute new url list
 
-    url = set(permission_obj['URL'])
+    new_urls = copy.copy(existing_permission["urls"])
 
-    if add_url:
-        for u in add_url:
-            domain = u[:u.index('/')]
-            path = u[u.index('/'):]
-            domain, path = _normalize_domain_path(domain, path)
-            url.add(domain + path)
-    if remove_url:
-        for u in remove_url:
-            domain = u[:u.index('/')]
-            path = u[u.index('/'):]
-            domain, path = _normalize_domain_path(domain, path)
-            url.discard(domain + path)
+    if add:
+        urls_to_add = [add] if not isinstance(add, list) else add
+        urls_to_add = [_normalize_url(url) for url in urls_to_add]
+        new_urls += urls_to_add
+    if remove:
+        urls_to_remove = [remove] if not isinstance(remove, list) else remove
+        urls_to_remove = [_normalize_url(url) for url in urls_to_remove]
+        new_urls = [u for u in new_urls if u not in urls_to_remove]
 
-    if url == set(permission_obj['URL']):
+    if set(new_urls) == set(existing_permission["urls"]):
         logger.warning(m18n.n('permission_update_nothing_to_do'))
-        return user_permission_list(app, permission)
+        return existing_permission
+
+    # Actually commit the change
 
     operation_logger.start()
-    if ldap.update('cn=%s,ou=permission' % permission_name, {'cn': permission_name, 'URL': url}):
+    if ldap.update('cn=%s,ou=permission' % permission, {'URL': new_urls}):
         if sync_perm:
             permission_sync_to_user()
-        logger.debug(m18n.n('permission_updated', permission=permission, app=app))
-        return user_permission_list(app, permission)
-
-    raise YunohostError('premission_update_failed')
+        logger.debug(m18n.n('permission_updated', permission=permission))
+        return user_permission_list(full=True)["permissions"][permission]
+    else:
+        raise YunohostError('premission_update_failed')
 
 
 @is_unit_operation(['permission', 'app'])
-def permission_delete(operation_logger, app, permission, force=False, sync_perm=True):
+def permission_delete(operation_logger, permission, force=False, sync_perm=True):
     """
-    Remove a permission for a specific application
+    Delete a permission
 
     Keyword argument:
-        app        -- an application OR sftp, xmpp (metronome), mail
-        permission -- name of the permission ("main" by default)
-
+        permission -- Name of the permission (e.g. nextcloud.main or wordpress.editors)
     """
 
-    if permission == "main" and not force:
+    if permission.endswith("main") and not force:
         raise YunohostError('remove_main_permission_not_allowed')
 
     from yunohost.utils.ldap import _get_ldap_interface
     ldap = _get_ldap_interface()
 
+    # Make sure this permission exists
+
+    existing_permission = user_permission_list(full=True)["permissions"].get(permission, None)
+    if not existing_permission:
+        raise YunohostError('permission_not_found', permission=permission)
+
+    # Actually delete the permission
+
     operation_logger.start()
-    if not ldap.remove('cn=%s,ou=permission' % str(permission + '.' + app)):
-        raise YunohostError('permission_deletion_failed', permission=permission, app=app)
-    if sync_perm:
-        permission_sync_to_user()
-    logger.debug(m18n.n('permission_deleted', permission=permission, app=app))
+    if ldap.remove('cn=%s,ou=permission' % permission):
+        if sync_perm:
+            permission_sync_to_user()
+        logger.debug(m18n.n('permission_deleted', permission=permission))
+    else:
+        raise YunohostError('permission_deletion_failed', permission=permission)
 
 
 def permission_sync_to_user(force=False):
@@ -438,3 +429,10 @@ def permission_sync_to_user(force=False):
     # Reload unscd, otherwise the group ain't propagated to the LDAP database
     os.system('nscd --invalidate=passwd')
     os.system('nscd --invalidate=group')
+
+def _normalize_url(url):
+    from yunohost.domain import _normalize_domain_path
+    domain = url[:url.index('/')]
+    path = url[url.index('/'):]
+    domain, path = _normalize_domain_path(domain, path)
+    return domain + path

From 853c6a161a07ca2a935b1eeb3424094f1f8e6a7c Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Wed, 11 Sep 2019 20:56:56 +0200
Subject: [PATCH 17/45] Simplify permission_sync_to_user ... force is never set
 to True so I dropped it...

---
 src/yunohost/permission.py | 71 +++++++++++---------------------------
 1 file changed, 21 insertions(+), 50 deletions(-)

diff --git a/src/yunohost/permission.py b/src/yunohost/permission.py
index 960c0853c..54aa25e23 100644
--- a/src/yunohost/permission.py
+++ b/src/yunohost/permission.py
@@ -358,70 +358,41 @@ def permission_delete(operation_logger, permission, force=False, sync_perm=True)
         raise YunohostError('permission_deletion_failed', permission=permission)
 
 
-def permission_sync_to_user(force=False):
+def permission_sync_to_user():
     """
     Sychronise the inheritPermission attribut in the permission object from the
     user<->group link and the group<->permission link
-
-    Keyword argument:
-        force    -- Force to recreate all attributes. Used generally with the
-        backup which uses "slapadd" which doesnt' use the memberOf overlay.
-        Note that by removing all value and adding a new time, we force the
-        overlay to update all attributes
     """
-    # Note that a LDAP operation with the same value that is in LDAP crash SLAP.
-    # So we need to check before each ldap operation that we really change something in LDAP
     import os
     from yunohost.app import app_ssowatconf
+    from yunohost.user import user_group_list
     from yunohost.utils.ldap import _get_ldap_interface
     ldap = _get_ldap_interface()
 
-    permission_attrs = [
-        'cn',
-        'member',
-    ]
-    group_info = ldap.search('ou=groups,dc=yunohost,dc=org',
-                             '(objectclass=groupOfNamesYnh)', permission_attrs)
-    group_info = {g['cn'][0]: g for g in group_info}
+    groups = user_group_list(full=True)["groups"]
+    permissions = user_permission_list(full=True)["permissions"]
 
-    for per in ldap.search('ou=permission,dc=yunohost,dc=org',
-                           '(objectclass=permissionYnh)',
-                           ['cn', 'inheritPermission', 'groupPermission', 'memberUid']):
+    for permission_name, permission_infos in permissions.items():
 
-        if 'groupPermission' not in per:
-            per['groupPermission'] = []
-        user_permission = set()
-        for group in per['groupPermission']:
-            group = group.split("=")[1].split(",")[0]
-            if 'member' not in group_info[group]:
-                continue
-            for user in group_info[group]['member']:
-                user_permission.add(user)
+        # These are the users currently allowed because there's an 'inheritPermission' object corresponding to it
+        currently_allowed_users = set(permission_infos["corresponding_users"])
 
-        if 'inheritPermission' not in per:
-            per['inheritPermission'] = []
-        if 'memberUid' not in per:
-            per['memberUid'] = []
+        # These are the users that should be allowed because they are member of a group that is allowed for this permission ...
+        should_be_allowed_users = set([user for group in permission_infos["allowed"] for user in groups[group]["members"]])
 
-        uid_val = [v.split("=")[1].split(",")[0] for v in user_permission]
-        if user_permission == set(per['inheritPermission']) and set(uid_val) == set(per['memberUid']) and not force:
+        # Note that a LDAP operation with the same value that is in LDAP crash SLAP.
+        # So we need to check before each ldap operation that we really change something in LDAP
+        if currently_allowed_users == should_be_allowed_users:
+            # We're all good, this permission is already correctly synchronized !
             continue
-        inheritPermission = {'inheritPermission': user_permission, 'memberUid': uid_val}
-        if force:
-            if per['groupPermission']:
-                if not ldap.update('cn=%s,ou=permission' % per['cn'][0], {'groupPermission': []}):
-                    raise YunohostError('permission_update_failed_clear')
-                if not ldap.update('cn=%s,ou=permission' % per['cn'][0], {'groupPermission': per['groupPermission']}):
-                    raise YunohostError('permission_update_failed_populate')
-            if per['inheritPermission']:
-                if not ldap.update('cn=%s,ou=permission' % per['cn'][0], {'inheritPermission': []}):
-                    raise YunohostError('permission_update_failed_clear')
-            if user_permission:
-                if not ldap.update('cn=%s,ou=permission' % per['cn'][0], inheritPermission):
-                    raise YunohostError('permission_update_failed')
-        else:
-            if not ldap.update('cn=%s,ou=permission' % per['cn'][0], inheritPermission):
-                raise YunohostError('permission_update_failed')
+
+        new_inherited_perms = {'inheritPermission': ["uid=%s,ou=users,dc=yunohost,dc=org" % u for u in should_be_allowed_users],
+                               'memberUid': should_be_allowed_users}
+
+        # Commit the change with the new inherited stuff
+        if not ldap.update('cn=%s,ou=permission' % permission_name, new_inherited_perms):
+            raise YunohostError('permission_update_failed')
+
     logger.debug(m18n.n('permission_generated'))
 
     app_ssowatconf()

From 98b1c30330af31c7e4bd9e81a0a436f1a3b3c5e2 Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Wed, 11 Sep 2019 21:40:04 +0200
Subject: [PATCH 18/45] Simplify app_ssowatconf code related to permissions

---
 src/yunohost/app.py | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/src/yunohost/app.py b/src/yunohost/app.py
index c2fb87acf..1a41e2a01 100644
--- a/src/yunohost/app.py
+++ b/src/yunohost/app.py
@@ -1414,12 +1414,10 @@ def app_ssowatconf():
     skipped_regex.append("^[^/]*/%.well%-known/acme%-challenge/.*$")
     skipped_regex.append("^[^/]*/%.well%-known/autoconfig/mail/config%-v1%.1%.xml.*$")
 
-    permission = {}
-    for a in user_permission_list()['permissions'].values():
-        for p in a.values():
-            if 'URL' in p:
-                for u in p['URL']:
-                    permission[u] = p['allowed_users']
+    permissions_per_url = {}
+    for permission_name, permission_infos in user_permission_list(full=True)['permissions'].items():
+        for url in permission_infos["urls"]:
+            permissions_per_url[url] = permission_infos['corresponding_users']
 
     conf_dict = {
         'portal_domain': main_domain,
@@ -1441,7 +1439,7 @@ def app_ssowatconf():
         'redirected_regex': redirected_regex,
         'users': {username: app_map(user=username)
                   for username in user_list()['users'].keys()},
-        'permission': permission,
+        'permissions': permissions_per_url,
     }
 
     with open('/etc/ssowat/conf.json', 'w+') as f:

From 38c43f4b9a6f899985fb7bb67d1c9ce7b1cafad9 Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Wed, 11 Sep 2019 22:48:54 +0200
Subject: [PATCH 19/45] Fix the whole operation logger / related to thing +
 propagate on the legacy addaccess

---
 locales/en.json            | 13 +++++--------
 src/yunohost/app.py        | 37 +++++++++++++++++++------------------
 src/yunohost/log.py        |  4 ++--
 src/yunohost/permission.py | 31 +++++++++++++++++++++++--------
 src/yunohost/user.py       | 26 +++++++++++++-------------
 5 files changed, 62 insertions(+), 49 deletions(-)

diff --git a/locales/en.json b/locales/en.json
index ebbb89fa8..2c6363608 100644
--- a/locales/en.json
+++ b/locales/en.json
@@ -259,9 +259,6 @@
     "log_help_to_get_failed_log": "The operation '{desc}' has failed! To get help, please share the full log of this operation using the command 'yunohost log display {name} --share'",
     "log_does_exists": "There is not operation log with the name '{log}', use 'yunohost log list to see all available operation logs'",
     "log_operation_unit_unclosed_properly": "Operation unit has not been closed properly",
-    "log_app_addaccess": "Add access to '{}'",
-    "log_app_removeaccess": "Remove access to '{}'",
-    "log_app_clearaccess": "Remove all access to '{}'",
     "log_app_fetchlist": "Add an application list",
     "log_app_removelist": "Remove an application list",
     "log_app_change_url": "Change the url of '{}' application",
@@ -279,9 +276,9 @@
     "log_dyndns_subscribe": "Subscribe to a YunoHost subdomain '{}'",
     "log_dyndns_update": "Update the ip associated with your YunoHost subdomain '{}'",
     "log_letsencrypt_cert_install": "Install Let's encrypt certificate on '{}' domain",
-    "log_permission_add": "Add permission '{}' for app '{}'",
-    "log_permission_remove": "Remove permission '{}'",
-    "log_permission_update": "Update permission '{}' for app '{}'",
+    "log_permission_create": "Create permission '{permission}'",
+    "log_permission_delete": "Delete permission '{permission}'",
+    "log_permission_urls": "Update urls related to permission '{permission}'",
     "log_selfsigned_cert_install": "Install self signed certificate on '{}' domain",
     "log_letsencrypt_cert_renew": "Renew '{}' Let's encrypt certificate",
     "log_regen_conf": "Regenerate system configurations '{}'",
@@ -291,8 +288,8 @@
     "log_user_group_delete": "Delete '{}' group",
     "log_user_group_update": "Update '{}' group",
     "log_user_update": "Update information of '{}' user",
-    "log_user_permission_add": "Update '{}' permission",
-    "log_user_permission_remove": "Update '{}' permission",
+    "log_user_permission_update": "Update accesses for permission '{permission}'",
+    "log_user_permission_reset": "Reset permission '{permission}'",
     "log_tools_maindomain": "Make '{}' as main domain",
     "log_tools_migrations_migrate_forward": "Migrate forward",
     "log_tools_postinstall": "Postinstall your YunoHost server",
diff --git a/src/yunohost/app.py b/src/yunohost/app.py
index 1a41e2a01..14396b1cd 100644
--- a/src/yunohost/app.py
+++ b/src/yunohost/app.py
@@ -1039,8 +1039,7 @@ def app_remove(operation_logger, app):
         raise YunohostError("this_action_broke_dpkg")
 
 
-@is_unit_operation(['permission','app'])
-def app_addaccess(operation_logger, apps, users=[]):
+def app_addaccess(apps, users=[]):
     """
     Grant access right to users (everyone by default)
 
@@ -1051,15 +1050,15 @@ def app_addaccess(operation_logger, apps, users=[]):
     """
     from yunohost.permission import user_permission_update
 
-    permission = user_permission_update(operation_logger, app=apps, permission="main", add_username=users)
+    output = {}
+    for app in apps:
+        permission = user_permission_update(app+".main", add=users)
+        output[app] = permission["corresponding_users"]
 
-    result = {p : v['main']['allowed_users'] for p, v in permission['permissions'].items()}
-
-    return {'allowed_users': result}
+    return {'allowed_users': output}
 
 
-@is_unit_operation(['permission','app'])
-def app_removeaccess(operation_logger, apps, users=[]):
+def app_removeaccess(apps, users=[]):
     """
     Revoke access right to users (everyone by default)
 
@@ -1070,15 +1069,15 @@ def app_removeaccess(operation_logger, apps, users=[]):
     """
     from yunohost.permission import user_permission_update
 
-    permission = user_permission_update(operation_logger, app=apps, permission="main", del_username=users)
+    output = {}
+    for app in apps:
+        permission = user_permission_update(app+".main", remove=users)
+        output[app] = permission["corresponding_users"]
 
-    result = {p : v['main']['allowed_users'] for p, v in permission['permissions'].items()}
-
-    return {'allowed_users': result}
+    return {'allowed_users': output}
 
 
-@is_unit_operation(['permission','app'])
-def app_clearaccess(operation_logger, apps):
+def app_clearaccess(apps):
     """
     Reset access rights for the app
 
@@ -1086,13 +1085,15 @@ def app_clearaccess(operation_logger, apps):
         apps
 
     """
-    from yunohost.permission import user_permission_clear
+    from yunohost.permission import user_permission_reset
 
-    permission = user_permission_clear(operation_logger, app=apps, permission="main")
+    output = {}
+    for app in apps:
+        permission = user_permission_reset(app+".main")
+        output[app] = permission["corresponding_users"]
 
-    result = {p : v['main']['allowed_users'] for p, v in permission['permissions'].items()}
+    return {'allowed_users': output}
 
-    return {'allowed_users': result}
 
 def app_debug(app):
     """
diff --git a/src/yunohost/log.py b/src/yunohost/log.py
index cbb850e44..8b0f893e8 100644
--- a/src/yunohost/log.py
+++ b/src/yunohost/log.py
@@ -44,7 +44,7 @@ CATEGORIES = ['operation', 'history', 'package', 'system', 'access', 'service',
               'app']
 METADATA_FILE_EXT = '.yml'
 LOG_FILE_EXT = '.log'
-RELATED_CATEGORIES = ['app', 'domain', 'service', 'user']
+RELATED_CATEGORIES = ['app', 'domain', 'group', 'service', 'user']
 
 logger = getActionLogger('yunohost.log')
 
@@ -213,7 +213,7 @@ def log_display(path, number=None, share=False):
     return infos
 
 
-def is_unit_operation(entities=['app', 'domain', 'service', 'user'],
+def is_unit_operation(entities=['app', 'domain', 'group', 'service', 'user'],
                       exclude=['password'], operation_key=None):
     """
     Configure quickly a unit operation
diff --git a/src/yunohost/permission.py b/src/yunohost/permission.py
index 54aa25e23..a4ff9fb15 100644
--- a/src/yunohost/permission.py
+++ b/src/yunohost/permission.py
@@ -76,6 +76,7 @@ def user_permission_list(short=False, full=False):
     return {'permissions': permissions}
 
 
+@is_unit_operation()
 def user_permission_update(operation_logger, permission, add=None, remove=None, sync_perm=True):
     """
     Allow or Disallow a user or group to a permission for a specific application
@@ -98,6 +99,7 @@ def user_permission_update(operation_logger, permission, add=None, remove=None,
 
     current_allowed_groups = existing_permission["allowed"]
     all_existing_groups = user_group_list()['groups'].keys()
+    operation_logger.related_to.append(('app', permission.split(".")[0]))
 
     # Compute new allowed group list (and make sure what we're doing make sense)
 
@@ -110,6 +112,8 @@ def user_permission_update(operation_logger, permission, add=None, remove=None,
                 raise YunohostError('group_unknown', group=group)
             if group in current_allowed_groups:
                 logger.warning(m18n.n('group_already_allowed', permission=permission, group=group))
+            else:
+                operation_logger.related_to.append(('group', group))
 
         new_allowed_groups += groups_to_add
 
@@ -120,6 +124,8 @@ def user_permission_update(operation_logger, permission, add=None, remove=None,
                 raise YunohostError('group_unknown', group=group)
             if group not in current_allowed_groups:
                 logger.warning(m18n.n('group_already_disallowed', permission=permission, group=group))
+            else:
+                operation_logger.related_to.append(('group', group))
 
         new_allowed_groups = [g for g in new_allowed_groups if g not in groups_to_remove]
 
@@ -132,15 +138,17 @@ def user_permission_update(operation_logger, permission, add=None, remove=None,
         # FIXME : write a better explanation ?
         logger.warning("This permission is currently enabled for all users in addition to other groups. You probably want to either remove the 'all_users' permission or remove the specific groups currently allowed.")
 
-    # Commit the new allowed group list
-
-    operation_logger.start()
-
     # Don't update LDAP if we update exactly the same values
     if set(new_allowed_groups) == set(current_allowed_groups):
         # FIXME : i18n
         logger.warning("No change was applied because not relevant modification were found")
-    elif ldap.update('cn=%s,ou=permission' % permission,
+        return
+
+    # Commit the new allowed group list
+
+    operation_logger.start()
+
+    if ldap.update('cn=%s,ou=permission' % permission,
                      {'groupPermission': ['cn=' + g + ',ou=groups,dc=yunohost,dc=org' for g in new_allowed_groups]}):
         logger.debug(m18n.n('permission_updated', permission=permission))
 
@@ -172,6 +180,7 @@ def user_permission_update(operation_logger, permission, add=None, remove=None,
         raise YunohostError('permission_update_failed')
 
 
+@is_unit_operation()
 def user_permission_reset(operation_logger, permission, sync_perm=True):
     """
     Reset a given permission to just 'all_users'
@@ -191,6 +200,9 @@ def user_permission_reset(operation_logger, permission, sync_perm=True):
 
     # Update permission with default (all_users)
 
+    operation_logger.related_to.append(('app', permission.split(".")[0]))
+    operation_logger.start()
+
     default_permission = {'groupPermission': ['cn=all_users,ou=groups,dc=yunohost,dc=org']}
     if ldap.update('cn=%s,ou=permission' % permission, default_permission):
         logger.debug(m18n.n('permission_updated', permission=permission))
@@ -228,7 +240,7 @@ def user_permission_reset(operation_logger, permission, sync_perm=True):
 #
 
 
-@is_unit_operation(['permission', 'app'])
+@is_unit_operation()
 def permission_create(operation_logger, permission, urls=None, sync_perm=True):
     """
     Create a new permission for a specific application
@@ -267,6 +279,7 @@ def permission_create(operation_logger, permission, urls=None, sync_perm=True):
     if urls:
         attr_dict['URL'] = [_normalize_url(url) for url in urls]
 
+    operation_logger.related_to.append(('app', permission.split(".")[0]))
     operation_logger.start()
     if ldap.add('cn=%s,ou=permission' % permission, attr_dict):
         if sync_perm:
@@ -277,7 +290,7 @@ def permission_create(operation_logger, permission, urls=None, sync_perm=True):
         raise YunohostError('permission_creation_failed')
 
 
-@is_unit_operation(['permission', 'app'])
+@is_unit_operation()
 def permission_urls(operation_logger, permission, add=None, remove=None, sync_perm=True):
     """
     Update urls related to a permission for a specific application
@@ -316,6 +329,7 @@ def permission_urls(operation_logger, permission, add=None, remove=None, sync_pe
 
     # Actually commit the change
 
+    operation_logger.related_to.append(('app', permission.split(".")[0]))
     operation_logger.start()
     if ldap.update('cn=%s,ou=permission' % permission, {'URL': new_urls}):
         if sync_perm:
@@ -326,7 +340,7 @@ def permission_urls(operation_logger, permission, add=None, remove=None, sync_pe
         raise YunohostError('premission_update_failed')
 
 
-@is_unit_operation(['permission', 'app'])
+@is_unit_operation()
 def permission_delete(operation_logger, permission, force=False, sync_perm=True):
     """
     Delete a permission
@@ -349,6 +363,7 @@ def permission_delete(operation_logger, permission, force=False, sync_perm=True)
 
     # Actually delete the permission
 
+    operation_logger.related_to.append(('app', permission.split(".")[0]))
     operation_logger.start()
     if ldap.remove('cn=%s,ou=permission' % permission):
         if sync_perm:
diff --git a/src/yunohost/user.py b/src/yunohost/user.py
index 2bf36cfd6..5631a2204 100644
--- a/src/yunohost/user.py
+++ b/src/yunohost/user.py
@@ -525,7 +525,7 @@ def user_group_list(short=False, full=False):
     return {'groups': groups}
 
 
-@is_unit_operation([('groupname', 'user')])
+@is_unit_operation([('groupname', 'group')])
 def user_group_create(operation_logger, groupname, gid=None, primary_group=False, sync_perm=True):
     """
     Create group
@@ -537,8 +537,6 @@ def user_group_create(operation_logger, groupname, gid=None, primary_group=False
     from yunohost.permission import permission_sync_to_user
     from yunohost.utils.ldap import _get_ldap_interface
 
-    operation_logger.start()
-
     ldap = _get_ldap_interface()
 
     # Validate uniqueness of groupname in LDAP
@@ -574,6 +572,7 @@ def user_group_create(operation_logger, groupname, gid=None, primary_group=False
     if primary_group:
         attr_dict["member"] = ["uid=" + groupname + ",ou=users,dc=yunohost,dc=org"]
 
+    operation_logger.start()
     if ldap.add('cn=%s,ou=groups' % groupname, attr_dict):
         logger.success(m18n.n('group_created', group=groupname))
         if sync_perm:
@@ -583,7 +582,7 @@ def user_group_create(operation_logger, groupname, gid=None, primary_group=False
     raise YunohostError('group_creation_failed', group=groupname)
 
 
-@is_unit_operation([('groupname', 'user')])
+@is_unit_operation([('groupname', 'group')])
 def user_group_delete(operation_logger, groupname, force=False, sync_perm=True):
     """
     Delete user
@@ -614,7 +613,7 @@ def user_group_delete(operation_logger, groupname, force=False, sync_perm=True):
         permission_sync_to_user()
 
 
-@is_unit_operation([('groupname', 'user')])
+@is_unit_operation([('groupname', 'group')])
 def user_group_update(operation_logger, groupname, add=None, remove=None, force=False, sync_perm=True):
     """
     Update user informations
@@ -650,6 +649,8 @@ def user_group_update(operation_logger, groupname, add=None, remove=None, force=
 
             if user in current_group:
                 logger.warning(m18n.n('user_already_in_group', user=user, group=groupname))
+            else:
+                operation_logger.related_to.append(('user', user))
 
         new_group += users_to_add
 
@@ -659,6 +660,8 @@ def user_group_update(operation_logger, groupname, add=None, remove=None, force=
         for user in users_to_remove:
             if user not in current_group:
                 logger.warning(m18n.n('user_not_in_group', user=user, group=groupname))
+            else:
+                operation_logger.related_to.append(('user', user))
 
         # Remove users_to_remove from new_group
         # Kinda like a new_group -= users_to_remove
@@ -666,9 +669,8 @@ def user_group_update(operation_logger, groupname, add=None, remove=None, force=
 
     new_group_dns = ["uid=" + user + ",ou=users,dc=yunohost,dc=org" for user in new_group]
 
-    operation_logger.start()
-
     if set(new_group) != set(current_group):
+        operation_logger.start()
         ldap = _get_ldap_interface()
         if not ldap.update('cn=%s,ou=groups' % groupname, {"member": set(new_group_dns), "memberUid": set(new_group)}):
             raise YunohostError('group_update_failed', group=groupname)
@@ -718,18 +720,16 @@ def user_permission_list(short=False, full=False):
     return yunohost.permission.user_permission_list(short, full)
 
 
-@is_unit_operation([('permission', 'user')])
-def user_permission_update(operation_logger, permission, add=None, remove=None, sync_perm=True):
+def user_permission_update(permission, add=None, remove=None, sync_perm=True):
     import yunohost.permission
-    return yunohost.permission.user_permission_update(operation_logger, permission,
+    return yunohost.permission.user_permission_update(permission,
                                                       add=add, remove=remove,
                                                       sync_perm=sync_perm)
 
 
-@is_unit_operation([('app', 'user')])
-def user_permission_reset(operation_logger, permission, sync_perm=True):
+def user_permission_reset(permission, sync_perm=True):
     import yunohost.permission
-    return yunohost.permission.user_permission_reset(operation_logger, permission,
+    return yunohost.permission.user_permission_reset(permission,
                                                      sync_perm=sync_perm)
 
 

From d5b2fb7a7126e80492d38e455bcb41f11e0b32b8 Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Wed, 11 Sep 2019 23:18:23 +0200
Subject: [PATCH 20/45] Misc fixes/improvements for i18n strings

---
 locales/en.json            | 38 ++++++++++++++++++--------------------
 src/yunohost/permission.py | 16 ++++++++--------
 src/yunohost/user.py       |  8 ++++----
 3 files changed, 30 insertions(+), 32 deletions(-)

diff --git a/locales/en.json b/locales/en.json
index 2c6363608..e41250759 100644
--- a/locales/en.json
+++ b/locales/en.json
@@ -228,19 +228,19 @@
     "global_settings_unknown_type": "Unexpected situation, the setting {setting:s} appears to have the type {unknown_type:s} but it's not a type supported by the system.",
     "good_practices_about_admin_password": "You are now about to define a new administration password. The password should be at least 8 characters - though it is good practice to use longer password (i.e. a passphrase) and/or to use various kind of characters (uppercase, lowercase, digits and special characters).",
     "good_practices_about_user_password": "You are now about to define a new user password. The password should be at least 8 characters - though it is good practice to use longer password (i.e. a passphrase) and/or to use various kind of characters (uppercase, lowercase, digits and special characters).",
-    "group_already_allowed": "Group '{group:s}' already has permission '{permission:s}' enabled'",
-    "group_already_disallowed": "Group '{group:s}' already has permission '{permission:s}' disabled'",
-    "group_name_already_exist": "Group {name:s} already exist",
+    "group_already_exist": "Group {group} already exist",
+    "group_already_exist_on_system": "Group {group} already exists in the system group",
     "group_created": "Group '{group}' successfully created",
     "group_creation_failed": "Group creation failed for group '{group}'",
-    "group_cannot_be_edited": "The group {group:s} cannot be edited manually.",
-    "group_cannot_be_deleted": "The group {group:s} cannot be deleted manually.",
+    "group_cannot_be_edited": "The group {group} cannot be edited manually.",
+    "group_cannot_be_deleted": "The group {group} cannot be deleted manually.",
     "group_deleted": "Group '{group}' deleted",
     "group_deletion_failed": "Group '{group} 'deletion failed",
-    "group_info_failed": "Group info failed",
-    "group_unknown": "Group {group:s} unknown",
+    "group_unknown": "Group {group} unknown",
     "group_updated": "Group '{group}' updated",
     "group_update_failed": "Group update failed for group '{group}'",
+    "group_user_already_in_group": "User {user} is already in group {group}",
+    "group_user_not_in_group": "User {user} is not in group {group}",
     "hook_exec_failed": "Script execution failed: {path:s}",
     "hook_exec_not_terminated": "Script execution did not finish properly: {path:s}",
     "hook_json_return_error": "Failed to read return from hook {path:s}. Error: {msg:s}. Raw content: {raw_content}",
@@ -426,22 +426,23 @@
     "pattern_positive_number": "Must be a positive number",
     "pattern_username": "Must be lower-case alphanumeric and underscore characters only",
     "pattern_password_app": "Sorry, passwords should not contain the following characters: {forbidden_chars}",
-    "permission_already_exist": "Permission '{permission:s}' for app {app:s} already exist",
-    "permission_created": "Permission '{permission:s}' for app {app:s} created",
-    "permission_creation_failed": "Permission creation failed",
-    "permission_deleted": "Permission '{permission:s}' for app {app:s} deleted",
-    "permission_deletion_failed": "Permission '{permission:s}' for app {app:s} deletion failed",
-    "permission_not_found": "Permission '{permission:s}' not found for application {app:s}",
-    "permission_update_failed": "Permission update failed",
-    "permission_generated": "The permission database has been updated",
-    "permission_updated": "Permission '{permission:s}' for app {app:s} updated",
+    "permission_already_allowed": "Group '{group}' already has permission '{permission}' enabled'",
+    "permission_already_disallowed": "Group '{group}' already has permission '{permission}' disabled'",
+    "permission_already_exist": "Permission '{permission}' already exists",
+    "permission_cannot_remove_main": "Removing a main permission is not allowed",
+    "permission_created": "Permission '{permission}' created",
+    "permission_creation_failed": "Failed to create permission '{permission}'",
+    "permission_deleted": "Permission '{permission}' deleted",
+    "permission_deletion_failed": "Failed to delete permission '{permission}'",
+    "permission_not_found": "Permission '{permission}' does not seem to exist ?",
+    "permission_update_failed": "Failed to update permission '{permission}'",
+    "permission_updated": "Permission '{permission}' updated",
     "permission_update_nothing_to_do": "No permissions to update",
     "port_already_closed": "Port {port:d} is already closed for {ip_version:s} connections",
     "port_already_opened": "Port {port:d} is already opened for {ip_version:s} connections",
     "port_available": "Port {port:d} is available",
     "port_unavailable": "Port {port:d} is not available",
     "recommend_to_add_first_user": "The post-install is finished but YunoHost needs at least one user to work correctly, you should add one using 'yunohost user create <username>' or the admin interface.",
-    "remove_main_permission_not_allowed": "Removing the main permission is not allowed",
     "regenconf_file_backed_up": "The configuration file '{conf}' has been backed up to '{backup}'",
     "regenconf_file_copy_failed": "Unable to copy the new configuration file '{new}' to '{conf}'",
     "regenconf_file_kept_back": "The configuration file '{conf}' is expected to be deleted by regen-conf (category {category}) but has been kept back.",
@@ -527,7 +528,6 @@
     "ssowat_conf_updated": "The SSOwat configuration has been updated",
     "ssowat_persistent_conf_read_error": "Error while reading SSOwat persistent configuration: {error:s}. Edit /etc/ssowat/conf.json.persistent file to fix the JSON syntax",
     "ssowat_persistent_conf_write_error": "Error while saving SSOwat persistent configuration: {error:s}. Edit /etc/ssowat/conf.json.persistent file to fix the JSON syntax",
-    "system_groupname_exists": "Groupname already exists in the system group",
     "system_upgraded": "The system has been upgraded",
     "system_username_exists": "Username already exists in the system users",
     "this_action_broke_dpkg": "This action broke dpkg/apt (the system package managers)... You can try to solve this issue by connecting through SSH and running `sudo dpkg --configure -a`.",
@@ -556,14 +556,12 @@
     "upnp_disabled": "UPnP has been disabled",
     "upnp_enabled": "UPnP has been enabled",
     "upnp_port_open_failed": "Unable to open UPnP ports",
-    "user_already_in_group": "User {user:} already in group {group:s}",
     "user_created": "The user has been created",
     "user_creation_failed": "Unable to create user",
     "user_deleted": "The user has been deleted",
     "user_deletion_failed": "Unable to delete user",
     "user_home_creation_failed": "Unable to create user home folder",
     "user_info_failed": "Unable to retrieve user information",
-    "user_not_in_group": "User {user:s} not in group {group:s}",
     "user_unknown": "Unknown user: {user:s}",
     "user_update_failed": "Unable to update user",
     "user_updated": "The user has been updated",
diff --git a/src/yunohost/permission.py b/src/yunohost/permission.py
index a4ff9fb15..21ee960fa 100644
--- a/src/yunohost/permission.py
+++ b/src/yunohost/permission.py
@@ -111,7 +111,7 @@ def user_permission_update(operation_logger, permission, add=None, remove=None,
             if group not in all_existing_groups:
                 raise YunohostError('group_unknown', group=group)
             if group in current_allowed_groups:
-                logger.warning(m18n.n('group_already_allowed', permission=permission, group=group))
+                logger.warning(m18n.n('permission_already_allowed', permission=permission, group=group))
             else:
                 operation_logger.related_to.append(('group', group))
 
@@ -123,7 +123,7 @@ def user_permission_update(operation_logger, permission, add=None, remove=None,
             if group not in all_existing_groups:
                 raise YunohostError('group_unknown', group=group)
             if group not in current_allowed_groups:
-                logger.warning(m18n.n('group_already_disallowed', permission=permission, group=group))
+                logger.warning(m18n.n('permission_already_disallowed', permission=permission, group=group))
             else:
                 operation_logger.related_to.append(('group', group))
 
@@ -177,7 +177,7 @@ def user_permission_update(operation_logger, permission, add=None, remove=None,
         return new_permission
 
     else:
-        raise YunohostError('permission_update_failed')
+        raise YunohostError('permission_update_failed', permission=permission)
 
 
 @is_unit_operation()
@@ -207,7 +207,7 @@ def user_permission_reset(operation_logger, permission, sync_perm=True):
     if ldap.update('cn=%s,ou=permission' % permission, default_permission):
         logger.debug(m18n.n('permission_updated', permission=permission))
     else:
-        raise YunohostError('permission_update_failed')
+        raise YunohostError('permission_update_failed', permission=permission)
 
     if sync_perm:
         permission_sync_to_user()
@@ -337,7 +337,7 @@ def permission_urls(operation_logger, permission, add=None, remove=None, sync_pe
         logger.debug(m18n.n('permission_updated', permission=permission))
         return user_permission_list(full=True)["permissions"][permission]
     else:
-        raise YunohostError('premission_update_failed')
+        raise YunohostError('permission_update_failed', permission=permission)
 
 
 @is_unit_operation()
@@ -350,7 +350,7 @@ def permission_delete(operation_logger, permission, force=False, sync_perm=True)
     """
 
     if permission.endswith("main") and not force:
-        raise YunohostError('remove_main_permission_not_allowed')
+        raise YunohostError('permission_cannot_remove_main')
 
     from yunohost.utils.ldap import _get_ldap_interface
     ldap = _get_ldap_interface()
@@ -406,9 +406,9 @@ def permission_sync_to_user():
 
         # Commit the change with the new inherited stuff
         if not ldap.update('cn=%s,ou=permission' % permission_name, new_inherited_perms):
-            raise YunohostError('permission_update_failed')
+            raise YunohostError('permission_update_failed', permission=permission_name)
 
-    logger.debug(m18n.n('permission_generated'))
+    logger.debug("The permission database has been resynchronized")
 
     app_ssowatconf()
 
diff --git a/src/yunohost/user.py b/src/yunohost/user.py
index 5631a2204..e1719d3a6 100644
--- a/src/yunohost/user.py
+++ b/src/yunohost/user.py
@@ -544,12 +544,12 @@ def user_group_create(operation_logger, groupname, gid=None, primary_group=False
         'cn': groupname
     }, base_dn='ou=groups,dc=yunohost,dc=org')
     if conflict:
-        raise YunohostError('group_name_already_exist', name=groupname)
+        raise YunohostError('group_already_exist', group=groupname)
 
     # Validate uniqueness of groupname in system group
     all_existing_groupnames = {x.gr_name for x in grp.getgrall()}
     if groupname in all_existing_groupnames:
-        raise YunohostError('system_groupname_exists')
+        raise YunohostError('group_already_exist_on_system', group=groupname)
 
     if not gid:
         # Get random GID
@@ -648,7 +648,7 @@ def user_group_update(operation_logger, groupname, add=None, remove=None, force=
                 raise YunohostError('user_unknown', user=user)
 
             if user in current_group:
-                logger.warning(m18n.n('user_already_in_group', user=user, group=groupname))
+                logger.warning(m18n.n('group_user_already_in_group', user=user, group=groupname))
             else:
                 operation_logger.related_to.append(('user', user))
 
@@ -659,7 +659,7 @@ def user_group_update(operation_logger, groupname, add=None, remove=None, force=
 
         for user in users_to_remove:
             if user not in current_group:
-                logger.warning(m18n.n('user_not_in_group', user=user, group=groupname))
+                logger.warning(m18n.n('group_user_not_in_group', user=user, group=groupname))
             else:
                 operation_logger.related_to.append(('user', user))
 

From a92ff5307774d657b601051b31e0e52634472180 Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Thu, 12 Sep 2019 00:20:22 +0200
Subject: [PATCH 21/45] Propagate changes to other parts of the code relying on
 groups and permissions

---
 src/yunohost/app.py        | 29 +++++++-----------
 src/yunohost/backup.py     | 61 ++++++++++++++++----------------------
 src/yunohost/permission.py |  9 ++++--
 3 files changed, 44 insertions(+), 55 deletions(-)

diff --git a/src/yunohost/app.py b/src/yunohost/app.py
index 14396b1cd..f505dd088 100644
--- a/src/yunohost/app.py
+++ b/src/yunohost/app.py
@@ -465,7 +465,7 @@ def app_change_url(operation_logger, app, domain, path):
     """
     from yunohost.hook import hook_exec, hook_callback
     from yunohost.domain import _normalize_domain_path, _get_conflicting_apps
-    from yunohost.permission import permission_update
+    from yunohost.permission import permission_urls
 
     installed = _is_installed(app)
     if not installed:
@@ -555,7 +555,7 @@ def app_change_url(operation_logger, app, domain, path):
     app_setting(app, 'domain', value=domain)
     app_setting(app, 'path', value=path)
 
-    permission_update(app, permission="main", add_url=[domain+path], remove_url=[old_domain+old_path], sync_perm=True)
+    permission_urls(app+".main", add=[domain+path], remove=[old_domain+old_path], sync_perm=True)
 
     # avoid common mistakes
     if _run_service_command("reload", "nginx") is False:
@@ -738,7 +738,7 @@ def app_install(operation_logger, app, label=None, args=None, no_remove_on_failu
     from yunohost.utils.ldap import _get_ldap_interface
     from yunohost.hook import hook_add, hook_remove, hook_exec, hook_callback
     from yunohost.log import OperationLogger
-    from yunohost.permission import permission_create, permission_update, permission_delete, permission_sync_to_user
+    from yunohost.permission import permission_create, permission_urls, permission_delete, permission_sync_to_user
     ldap = _get_ldap_interface()
 
     # Fetch or extract sources
@@ -875,7 +875,7 @@ def app_install(operation_logger, app, label=None, args=None, no_remove_on_failu
     # Create permission before the install (useful if the install script redefine the permission)
     # Note that sync_perm is disabled to avoid triggering a whole bunch of code and messages
     # can't be sure that we don't have one case when it's needed
-    permission_create(app=app_instance_name, permission="main", sync_perm=False)
+    permission_create(app_instance_name+".main", sync_perm=False)
 
     # Execute the app install script
     install_retcode = 1
@@ -910,11 +910,9 @@ def app_install(operation_logger, app, label=None, args=None, no_remove_on_failu
                     args=[app_instance_name], env=env_dict_remove
                 )[0]
                 # Remove all permission in LDAP
-                result = ldap.search(base='ou=permission,dc=yunohost,dc=org',
-                                    filter='(&(objectclass=permissionYnh)(cn=*.%s))' % app_instance_name, attrs=['cn'])
-                permission_list = [p['cn'][0] for p in result]
-                for l in permission_list:
-                    permission_delete(app_instance_name, l.split('.')[0], force=True)
+                for permission_name in user_permission_list()["permissions"].keys():
+                    if permission_name.startswith(app_instance_name+"."):
+                        permission_delete(permission_name, force=True)
 
                 if remove_retcode != 0:
                     msg = m18n.n('app_not_properly_removed',
@@ -960,8 +958,7 @@ def app_install(operation_logger, app, label=None, args=None, no_remove_on_failu
     domain = app_settings.get('domain', None)
     path = app_settings.get('path', None)
     if domain and path:
-        permission_update(app_instance_name, permission="main", add_url=[domain+path], sync_perm=False)
-
+        permission_urls(app_instance_name+".main", add=[domain+path], sync_perm=False)
     permission_sync_to_user()
 
     logger.success(m18n.n('installation_complete'))
@@ -978,7 +975,6 @@ def app_remove(operation_logger, app):
         app -- App(s) to delete
 
     """
-    from yunohost.utils.ldap import _get_ldap_interface
     from yunohost.hook import hook_exec, hook_remove, hook_callback
     from yunohost.permission import permission_delete, permission_sync_to_user
     if not _is_installed(app):
@@ -1026,12 +1022,9 @@ def app_remove(operation_logger, app):
     hook_remove(app)
 
     # Remove all permission in LDAP
-    ldap = _get_ldap_interface()
-    result = ldap.search(base='ou=permission,dc=yunohost,dc=org',
-                         filter='(&(objectclass=permissionYnh)(cn=*.%s))' % app, attrs=['cn'])
-    permission_list = [p['cn'][0] for p in result]
-    for l in permission_list:
-        permission_delete(app, l.split('.')[0], force=True, sync_perm=False)
+    for permission_name in user_permission_list()["permissions"].keys():
+        if permission_name.startswith(app+"."):
+            permission_delete(permission_name, force=True, sync_perm=False)
 
     permission_sync_to_user()
 
diff --git a/src/yunohost/backup.py b/src/yunohost/backup.py
index fdbd8c62c..0527f89bf 100644
--- a/src/yunohost/backup.py
+++ b/src/yunohost/backup.py
@@ -703,6 +703,10 @@ class BackupManager():
             self._import_to_list_to_backup(env_dict["YNH_BACKUP_CSV"])
 
             # backup permissions
+            #
+            # FIXME : why can't we instead use a simple yaml file to store the
+            # relevant info and recreate the permission object from it ...
+            #
             logger.debug(m18n.n('backup_permission', app=app))
             ldap_url = "ldap:///dc=yunohost,dc=org???(&(objectClass=permissionYnh)(cn=*.%s))" % app
             os.system("slapcat -b dc=yunohost,dc=org -H '%s' -l '%s/permission.ldif'" % (ldap_url, settings_dir))
@@ -919,7 +923,7 @@ class RestoreManager():
 
         successfull_apps = self.targets.list("apps", include=["Success", "Warning"])
 
-        permission_sync_to_user(force=False)
+        permission_sync_to_user()
 
         if os.path.ismount(self.work_dir):
             ret = subprocess.call(["umount", self.work_dir])
@@ -1183,18 +1187,11 @@ class RestoreManager():
         if system_targets == []:
             return
 
-        from yunohost.utils.ldap import _get_ldap_interface
-        ldap = _get_ldap_interface()
+        from yunohost.permission import permission_create, user_permission_update, user_permission_list
 
         # Backup old permission for apps
         # We need to do that because in case of an app is installed we can't remove the permission for this app
-        old_apps_permission = []
-        try:
-            old_apps_permission = ldap.search('ou=permission,dc=yunohost,dc=org',
-                                              '(&(objectClass=permissionYnh)(!(cn=main.mail))(!(cn=main.xmpp))(!(cn=main.sftp)))',
-                                              ['cn', 'objectClass', 'groupPermission', 'URL', 'gidNumber'])
-        except:
-            logger.info(m18n.n('apps_permission_not_found'))
+        old_apps_permission = user_permission_list(ignore_system_perms=True)["permissions"]
 
         # Start register change on system
         operation_logger = OperationLogger('backup_restore_system')
@@ -1232,12 +1229,11 @@ class RestoreManager():
 
         regen_conf()
 
-        # Check if we need to do the migration 0009 : setup group and permission
+        # Check that at least a group exists (all_users) to know if we need to
+        # do the migration 0011 : setup group and permission
+        #
         # Legacy code
-        result = ldap.search('ou=groups,dc=yunohost,dc=org',
-                             '(&(objectclass=groupOfNamesYnh)(cn=all_users))',
-                             ['cn'])
-        if not result:
+        if not user_group_list["groups"]:
             from yunohost.tools import _get_migration_by_name
             setup_group_permission = _get_migration_by_name("setup_group_permission")
             # Update LDAP schema restart slapd
@@ -1245,22 +1241,16 @@ class RestoreManager():
             regen_conf(names=['slapd'], force=True)
             setup_group_permission.migrate_LDAP_db()
 
-        # Remove all permission for all app which sill in the LDAP
-        for per in ldap.search('ou=permission,dc=yunohost,dc=org',
-                               '(&(objectClass=permissionYnh)(!(cn=mail.main))(!(cn=xmpp.main))(!(cn=sftp.main)))',
-                               ['cn']):
-            if not ldap.remove('cn=%s,ou=permission' % per['cn'][0]):
-                raise YunohostError('permission_deletion_failed',
-                                    permission=per['cn'][0].split('.')[0],
-                                    app=per['cn'][0].split('.')[1])
+        # Remove all permission for all app which is still in the LDAP
+        for permission_name in user_permission_list(ignore_system_perms=True)["permissions"].keys():
+            permission_delete(permission_name)
 
         # Restore permission for the app which is installed
-        for per in old_apps_permission:
-            # FIXME : will come here later to fix this following previous commits ...
-            permission_name, app_name = per['cn'][0].split('.')
+        for permission_name, permission_infos in old_apps_permission.items():
+            app_name = permission_name.split(".")[0]
             if _is_installed(app_name):
-                if not ldap.add('cn=%s,ou=permission' % per['cn'][0], per):
-                    raise YunohostError('apps_permission_restoration_failed', permission=permission_name, app=app_name)
+                permission_create(permission_name, urls=permission_infos["urls"], sync_perm=False)
+                user_permission_update(permission_name, remove="all_users", add=permission_infos["allowed"])
 
 
     def _restore_apps(self):
@@ -1368,6 +1358,10 @@ class RestoreManager():
             restore_script = os.path.join(tmp_folder_for_app_restore, 'restore')
 
             # Restore permissions
+            #
+            # FIXME : why can't we instead use a simple yaml file to store the
+            # relevant info and recreate the permission object from it ...
+            #
             if os.path.isfile(app_settings_in_archive + '/permission.ldif'):
                 filtred_entries =  ['entryUUID', 'creatorsName', 'createTimestamp', 'entryCSN', 'structuralObjectClass',
                                     'modifiersName', 'modifyTimestamp', 'inheritPermission', 'memberUid']
@@ -1422,7 +1416,6 @@ class RestoreManager():
             operation_logger.start()
 
             # Execute remove script
-            # TODO: call app_remove instead
             if hook_exec(remove_script, args=[app_instance_name],
                          env=env_dict_remove)[0] != 0:
                 msg = m18n.n('app_not_properly_removed', app=app_instance_name)
@@ -1434,12 +1427,10 @@ class RestoreManager():
             # Cleaning app directory
             shutil.rmtree(app_settings_new_path, ignore_errors=True)
 
-            # Remove all permission in LDAP
-            result = ldap.search(base='ou=permission,dc=yunohost,dc=org',
-                                 filter='(&(objectclass=permissionYnh)(cn=*.%s))' % app_instance_name, attrs=['cn'])
-            permission_list = [p['cn'][0] for p in result]
-            for l in permission_list:
-                permission_delete(app_instance_name, l.split('.')[0], force=True)
+            # Remove all permission in LDAP for this app
+            for permission_name in user_permission_list()["permissions"].keys():
+                if permission_name.startswith(app_instance_name+"."):
+                    permission_delete(permission_name, force=True)
 
             # TODO Cleaning app hooks
         else:
diff --git a/src/yunohost/permission.py b/src/yunohost/permission.py
index 21ee960fa..4d935d3c0 100644
--- a/src/yunohost/permission.py
+++ b/src/yunohost/permission.py
@@ -36,6 +36,8 @@ from yunohost.log import is_unit_operation
 
 logger = getActionLogger('yunohost.user')
 
+SYSTEM_PERMS = ["mail", "xmpp", "stfp"]
+
 #
 #
 #  The followings are the methods exposed through the "yunohost user permission" interface
@@ -43,7 +45,7 @@ logger = getActionLogger('yunohost.user')
 #
 
 
-def user_permission_list(short=False, full=False):
+def user_permission_list(short=False, full=False, ignore_system_perms=True):
     """
     List permissions and corresponding accesses
     """
@@ -62,8 +64,11 @@ def user_permission_list(short=False, full=False):
     for infos in permissions_infos:
 
         name = infos['cn'][0]
-        permissions[name] = {}
 
+        if ignore_system_perms and name.split(".")[0] in SYSTEM_PERMS:
+            continue
+
+        permissions[name] = {}
         permissions[name]["allowed"] = [_ldap_path_extract(p, "cn") for p in infos.get('groupPermission', [])]
 
         if full:

From bbfc62cf3efc298fc297fa4da52d45e420cc0c0c Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Thu, 12 Sep 2019 00:41:38 +0200
Subject: [PATCH 22/45] Backup/restore app permissions using yaml files which
 are much simpler to handle...

---
 locales/en.json        |  1 -
 src/yunohost/backup.py | 55 +++++++++++++++++++-----------------------
 2 files changed, 25 insertions(+), 31 deletions(-)

diff --git a/locales/en.json b/locales/en.json
index e41250759..e69e06201 100644
--- a/locales/en.json
+++ b/locales/en.json
@@ -50,7 +50,6 @@
     "app_upgrade_some_app_failed": "Unable to upgrade some applications",
     "app_upgraded": "{app:s} has been upgraded",
     "apps_permission_not_found": "No permission found for the installed apps",
-    "apps_permission_restoration_failed": "Permission '{permission:s}' for app {app:s} restoration has failed",
     "appslist_corrupted_json": "Could not load the application lists. It looks like {filename:s} is corrupted.",
     "appslist_could_not_migrate": "Could not migrate app list {appslist:s}! Unable to parse the url… The old cron job has been kept in {bkp_file:s}.",
     "appslist_fetched": "The application list {appslist:s} has been fetched",
diff --git a/src/yunohost/backup.py b/src/yunohost/backup.py
index 0527f89bf..f1ac7ee9c 100644
--- a/src/yunohost/backup.py
+++ b/src/yunohost/backup.py
@@ -40,7 +40,7 @@ from moulinette import msignals, m18n
 from yunohost.utils.error import YunohostError
 from moulinette.utils import filesystem
 from moulinette.utils.log import getActionLogger
-from moulinette.utils.filesystem import read_file, mkdir
+from moulinette.utils.filesystem import read_file, mkdir, write_to_yaml, read_yaml
 
 from yunohost.app import (
     app_info, _is_installed, _parse_app_instance_name, _patch_php5
@@ -677,6 +677,8 @@ class BackupManager():
         backup_app_failed -- Raised at the end if the app backup script
                              execution failed
         """
+        from yunohost.permission import user_permission_list
+
         app_setting_path = os.path.join('/etc/yunohost/apps/', app)
 
         # Prepare environment
@@ -703,13 +705,10 @@ class BackupManager():
             self._import_to_list_to_backup(env_dict["YNH_BACKUP_CSV"])
 
             # backup permissions
-            #
-            # FIXME : why can't we instead use a simple yaml file to store the
-            # relevant info and recreate the permission object from it ...
-            #
             logger.debug(m18n.n('backup_permission', app=app))
-            ldap_url = "ldap:///dc=yunohost,dc=org???(&(objectClass=permissionYnh)(cn=*.%s))" % app
-            os.system("slapcat -b dc=yunohost,dc=org -H '%s' -l '%s/permission.ldif'" % (ldap_url, settings_dir))
+            permissions = user_permission_list(full=True)["permissions"]
+            this_app_permissions = {name: infos for name, infos in permissions.items() if name.startswith(app + ".")}
+            write_to_yaml("%s/permissions.yml" % settings_dir, this_app_permissions)
 
         except:
             abs_tmp_app_dir = os.path.join(self.work_dir, 'apps/', app)
@@ -1289,11 +1288,8 @@ class RestoreManager():
                                         name already exists
         restore_app_failed -- Raised if the restore bash script failed
         """
-        from moulinette.utils.filesystem import read_ldif
         from yunohost.user import user_group_list
-        from yunohost.permission import permission_delete
-        from yunohost.utils.ldap import _get_ldap_interface
-        ldap = _get_ldap_interface()
+        from yunohost.permission import permission_create, permission_delete, user_permission_list, user_permission_update
 
         def copytree(src, dst, symlinks=False, ignore=None):
             for item in os.listdir(src):
@@ -1358,26 +1354,25 @@ class RestoreManager():
             restore_script = os.path.join(tmp_folder_for_app_restore, 'restore')
 
             # Restore permissions
-            #
-            # FIXME : why can't we instead use a simple yaml file to store the
-            # relevant info and recreate the permission object from it ...
-            #
-            if os.path.isfile(app_settings_in_archive + '/permission.ldif'):
-                filtred_entries =  ['entryUUID', 'creatorsName', 'createTimestamp', 'entryCSN', 'structuralObjectClass',
-                                    'modifiersName', 'modifyTimestamp', 'inheritPermission', 'memberUid']
-                entries = read_ldif('%s/permission.ldif' % app_settings_in_archive, filtred_entries)
-                group_list = user_group_list()['groups']
-                for dn, entry in entries:
-                    # Remove the group which has been removed
-                    for group in entry['groupPermission']:
-                        group_name = group.split(',')[0].split('=')[1]
-                        if group_name not in group_list:
-                            entry['groupPermission'].remove(group)
-                    if not ldap.add('cn=%s,ou=permission' % entry['cn'][0], entry):
-                        raise YunohostError('apps_permission_restoration_failed',
-                                            permission=entry['cn'][0].split('.')[0],
-                                            app=entry['cn'][0].split('.')[1])
+            if os.path.isfile('%s/permissions.yml' % app_settings_new_path):
+
+                permissions = read_yaml('%s/permissions.yml' % app_settings_new_path)
+                existing_groups = user_group_list()['groups']
+
+                for permission_name, permission_infos in permissions:
+
+                    permission_create(permission_name, urls=permission_infos.get("urls", []))
+
+                    if "allowed" not in permissions_infos:
+                        logger.warning("'allowed' key corresponding to allowed groups for permission %s not found when restoring app %s ... You might need to reconfigure permissions yourself!" % (permission_name, app_instance_name))
+                    else:
+                        groups = [g for g in permission_infos["allowed"] if g in existing_groups]
+                        user_permission_update(permission_name, remove="all_users", add=groups)
+
+                os.remove('%s/permissions.yml' % app_settings_new_path)
             else:
+                # Otherwise, we need to migrate the legacy permissions of this
+                # app (included in its settings.yml)
                 from yunohost.tools import _get_migration_by_name
                 setup_group_permission = _get_migration_by_name("setup_group_permission")
                 setup_group_permission.migrate_app_permission(app=app_instance_name)

From e40698ef2062346638a4492924a4dacf32f081f2 Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Thu, 12 Sep 2019 02:25:52 +0200
Subject: [PATCH 23/45] Propagate changes on migration

---
 locales/en.json                               |  2 +-
 .../0011_setup_group_permission.py            | 20 ++++++++-----------
 2 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/locales/en.json b/locales/en.json
index e69e06201..c370f821e 100644
--- a/locales/en.json
+++ b/locales/en.json
@@ -195,7 +195,6 @@
     "dyndns_registration_failed": "Unable to register DynDNS domain: {error:s}",
     "dyndns_domain_not_provided": "Dyndns provider {provider:s} cannot provide domain {domain:s}.",
     "dyndns_unavailable": "Domain {domain:s} is not available.",
-    "error_when_removing_sftpuser_group": "Error when trying remove sftpusers group",
     "executing_command": "Executing command '{command:s}'…",
     "executing_script": "Executing script '{script:s}'…",
     "extracting": "Extracting…",
@@ -355,6 +354,7 @@
     "migration_0011_can_not_backup_before_migration": "The backup of the system before the migration failed. Migration failed. Error: {error:s}",
     "migration_0011_create_group": "Creating a group for each user...",
     "migration_0011_done": "Migration successful. You are now able to manage groups of users.",
+    "migration_0011_error_when_removing_sftpuser_group": "Error when trying remove sftpusers group",
     "migration_0011_LDAP_config_dirty": "It look like that you customized your LDAP configuration. For this migration the LDAP configuration need to be updated.\nYou need to save your actual configuration, reintialize the original configuration by the command 'yunohost tools regen-conf -f' and after retry the migration",
     "migration_0011_LDAP_update_failed": "LDAP update failed. Error: {error:s}",
     "migration_0011_migrate_permission": "Migrating permissions from apps settings to LDAP...",
diff --git a/src/yunohost/data_migrations/0011_setup_group_permission.py b/src/yunohost/data_migrations/0011_setup_group_permission.py
index d2924f0af..720e4ac36 100644
--- a/src/yunohost/data_migrations/0011_setup_group_permission.py
+++ b/src/yunohost/data_migrations/0011_setup_group_permission.py
@@ -1,17 +1,16 @@
-import yaml
 import time
 import os
 
 from moulinette import m18n
 from yunohost.utils.error import YunohostError
 from moulinette.utils.log import getActionLogger
+from moulinette.utils.filesystem import read_yaml
 
 from yunohost.tools import Migration
 from yunohost.user import user_group_create, user_group_update
 from yunohost.app import app_setting, app_list
 from yunohost.regenconf import regen_conf
-from yunohost.permission import permission_create, permission_sync_to_user
-from yunohost.user import user_permission_add
+from yunohost.permission import permission_create, user_permission_update, permission_sync_to_user
 
 logger = getActionLogger('yunohost.migration')
 
@@ -19,6 +18,7 @@ logger = getActionLogger('yunohost.migration')
 # Tools used also for restoration
 ###################################################
 
+
 class MyMigration(Migration):
     """
         Update the LDAP DB to be able to store the permission
@@ -38,10 +38,9 @@ class MyMigration(Migration):
         try:
             ldap.remove('cn=sftpusers,ou=groups')
         except:
-            logger.warn(m18n.n("error_when_removing_sftpuser_group"))
+            logger.warn(m18n.n("migration_0011_error_when_removing_sftpuser_group"))
 
-        with open('/usr/share/yunohost/yunohost-config/moulinette/ldap_scheme.yml') as f:
-            ldap_map = yaml.load(f)
+        ldap_map = read_yaml('/usr/share/yunohost/yunohost-config/moulinette/ldap_scheme.yml')
 
         try:
             attr_dict = ldap_map['parents']['ou=permission']
@@ -65,11 +64,9 @@ class MyMigration(Migration):
             username = user_info['uid'][0]
             ldap.update('uid=%s,ou=users' % username,
                         {'objectClass': ['mailAccount', 'inetOrgPerson', 'posixAccount', 'userPermissionYnh']})
-            user_group_create(username, gid=user_info['uidNumber'][0], sync_perm=False)
-            user_group_update(groupname=username, add=username, force=True, sync_perm=False)
+            user_group_create(username, gid=user_info['uidNumber'][0], primary_group=True, sync_perm=False)
             user_group_update(groupname='all_users', add=username, force=True, sync_perm=False)
 
-
     def migrate_app_permission(self, app=None):
         logger.info(m18n.n("migration_0011_migrate_permission"))
 
@@ -85,13 +82,12 @@ class MyMigration(Migration):
             domain = app_setting(app, 'domain')
 
             urls = [domain + path] if domain and path else None
-            permission_create(app, permission='main', urls=urls, default_allow=True, sync_perm=False)
+            permission_create(app+".main", urls=urls, sync_perm=False)
             if permission:
                 allowed_group = permission.split(',')
-                user_permission_add([app], permission='main', group=allowed_group, sync_perm=False)
+                user_permission_update(app+".main", remove="all_users", add=allowed_group, sync_perm=False)
             app_setting(app, 'allowed_users', delete=True)
 
-
     def run(self):
         # Check if the migration can be processed
         ldap_regen_conf_status = regen_conf(names=['slapd'], dry_run=True)

From c0361430e26d42e0b8f167a330a20abff854655d Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Thu, 12 Sep 2019 04:02:03 +0200
Subject: [PATCH 24/45] Try to simplify + comment the code of
 check_LDAP_db_integrity

---
 src/yunohost/tests/test_permission.py | 79 +++++++++++++++------------
 1 file changed, 43 insertions(+), 36 deletions(-)

diff --git a/src/yunohost/tests/test_permission.py b/src/yunohost/tests/test_permission.py
index 8222248b1..0373a6cbf 100644
--- a/src/yunohost/tests/test_permission.py
+++ b/src/yunohost/tests/test_permission.py
@@ -2,8 +2,10 @@ 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_create, user_permission_list, user_delete, user_group_list, user_group_delete, user_permission_add, user_permission_remove, user_permission_clear
-from yunohost.permission import permission_create, permission_update, permission_delete
+
+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.permission import user_permission_update, user_permission_list, permission_create, permission_urls, permission_delete
 from yunohost.domain import _get_maindomain
 from yunohost.utils.error import YunohostError
 
@@ -57,7 +59,7 @@ def check_LDAP_db_integrity():
     # One part should be done automatically by the "memberOf" overlay of LDAP.
     # The other part is done by the the "permission_sync_to_user" function of the permission module
 
-    from yunohost.utils.ldap import _get_ldap_interface
+    from yunohost.utils.ldap import _get_ldap_interface, _ldap_path_extract
     ldap = _get_ldap_interface()
 
     user_search = ldap.search('ou=users,dc=yunohost,dc=org',
@@ -76,60 +78,65 @@ def check_LDAP_db_integrity():
 
     for user in user_search:
         user_dn = 'uid=' + user['uid'][0] + ',ou=users,dc=yunohost,dc=org'
-        group_list = [m.split("=")[1].split(",")[0] for m in user['memberOf']]
-        permission_list = []
-        if 'permission' in user:
-            permission_list = [m.split("=")[1].split(",")[0] for m in user['permission']]
+        group_list = [_ldap_path_extract(m, "cn") for m in user['memberOf']]
+        permission_list = [_ldap_path_extract(m, "cn") for m in user.get('permission', [])]
 
+        # This user's DN sould be found in all groups it is a member of
         for group in group_list:
             assert user_dn in group_map[group]['member']
+
+        # This user's DN should be found in all perms it has access to
         for permission in permission_list:
             assert user_dn in permission_map[permission]['inheritPermission']
 
     for permission in permission_search:
         permission_dn = 'cn=' + permission['cn'][0] + ',ou=permission,dc=yunohost,dc=org'
-        user_list = []
-        group_list = []
-        if 'inheritPermission' in permission:
-            user_list = [m.split("=")[1].split(",")[0] for m in permission['inheritPermission']]
-            assert set(user_list) == set(permission['memberUid'])
-        if 'groupPermission' in permission:
-            group_list = [m.split("=")[1].split(",")[0] for m in permission['groupPermission']]
 
+        # inheritPermission uid's should match memberUids
+        user_list = [_ldap_path_extract(m, "uid") for m in permission.get('inheritPermission', [])]
+        assert set(user_list) == set(permission.get('memberUid', []))
+
+        # This perm's DN should be found on all related users it is related to
         for user in user_list:
             assert permission_dn in user_map[user]['permission']
+
+        # Same for groups : we should find the permission's DN for all related groups
+        group_list = [_ldap_path_extract(m, "cn") for m in permission.get('groupPermission', [])]
         for group in group_list:
             assert permission_dn in group_map[group]['permission']
-            if 'member' in group_map[group]:
-                user_list_in_group = [m.split("=")[1].split(",")[0] for m in group_map[group]['member']]
-                assert set(user_list_in_group) <= set(user_list)
+
+            # The list of user in the group should be a subset of all users related to the current permission
+            users_in_group = [_ldap_path_extract(m, "uid") for m in group_map[group].get("member", [])]
+            assert set(users_in_group) <= set(user_list)
 
     for group in group_search:
         group_dn = 'cn=' + group['cn'][0] + ',ou=groups,dc=yunohost,dc=org'
-        user_list = []
-        permission_list = []
-        if 'member' in group:
-            user_list = [m.split("=")[1].split(",")[0] for m in group['member']]
-            if group['cn'][0] in user_list:
-                # If it's the main group of the user it's normal that it is not in the memberUid
-                g_list = list(user_list)
-                g_list.remove(group['cn'][0])
-                if 'memberUid' in group:
-                    assert set(g_list) == set(group['memberUid'])
-                else:
-                    assert g_list == []
-            else:
-                assert set(user_list) == set(group['memberUid'])
-        if 'permission' in group:
-            permission_list = [m.split("=")[1].split(",")[0] for m in group['permission']]
 
+        user_list = [_ldap_path_extract(m, "uid") for m in group.get("member", [])]
+        # For primary groups, we should find that :
+        #    - len(user_list) is 1 (a primary group has only 1 member)
+        #    - the group name should be an existing yunohost user
+        #    - memberUid is empty (meaning no other member than the corresponding user)
+        if group['cn'][0] in user_list:
+            assert len(user_list) == 1
+            assert group["cn"][0] in user_map
+            assert group.get('memberUid', []) == []
+        # Otherwise, user_list and memberUid should have the same content
+        else:
+            assert set(user_list) == set(group.get('memberUid', []))
+
+        # For all users members, this group should be in the "memberOf" on the other side
         for user in user_list:
             assert group_dn in user_map[user]['memberOf']
+
+        # For all the permissions of this group, the group should be among the "groupPermission" on the other side
+        permission_list = [_ldap_path_extract(m, "cn") for m in group.get('permission', [])]
         for permission in permission_list:
             assert group_dn in permission_map[permission]['groupPermission']
-            if 'inheritPermission' in permission_map:
-                allowed_user_list = [m.split("=")[1].split(",")[0] for m in permission_map[permission]['inheritPermission']]
-                assert set(user_list) <= set(allowed_user_list)
+
+            # And the list of user of this group (user_list) should be a subset of all allowed users for this perm...
+            allowed_user_list = [_ldap_path_extract(m, "uid") for m in permission_map[permission].get('inheritPermission', [])]
+            assert set(user_list) <= set(allowed_user_list)
 
 
 def check_permission_for_apps():

From f1f65137965039bca3cc4d7b531b10babfadcd61 Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Thu, 12 Sep 2019 04:02:36 +0200
Subject: [PATCH 25/45] Small tweaks for user group tests

---
 src/yunohost/tests/test_user-group.py | 86 ++++++++++++---------------
 1 file changed, 39 insertions(+), 47 deletions(-)

diff --git a/src/yunohost/tests/test_user-group.py b/src/yunohost/tests/test_user-group.py
index 34e515ea0..88644c3e6 100644
--- a/src/yunohost/tests/test_user-group.py
+++ b/src/yunohost/tests/test_user-group.py
@@ -1,7 +1,8 @@
 import pytest
 
 from moulinette.core import MoulinetteError
-from yunohost.user import user_list, user_info, user_group_list, user_create, user_delete, user_update, user_group_create, user_group_delete, user_group_update, user_group_info
+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
 from yunohost.utils.error import YunohostError
 from yunohost.tests.test_permission import check_LDAP_db_integrity
@@ -82,12 +83,13 @@ def test_del_user():
     assert "alice" not in group_res
     assert "alice" not in group_res['all_users']['members']
 
-def test_add_group():
+def test_create_group():
     user_group_create("adminsys")
 
     group_res = user_group_list()['groups']
     assert "adminsys" in group_res
-    assert "members" not in group_res['adminsys']
+    assert "members" in group_res['adminsys'].keys()
+    assert group_res["adminsys"]["members"] == []
 
 def test_del_group():
     user_group_delete("dev")
@@ -99,112 +101,102 @@ def test_del_group():
 # Error on create / remove function
 #
 
-def test_add_bad_user_1():
-    # Check email already exist
+def test_create_user_with_mail_address_already_taken():
     with pytest.raises(MoulinetteError):
         user_create("alice2", "Alice", "White", "alice@" + maindomain, "test123Ynh")
 
-def test_add_bad_user_2():
-    # Check to short password
+def test_create_user_with_password_too_simple():
     with pytest.raises(MoulinetteError):
         user_create("other", "Alice", "White", "other@" + maindomain, "12")
 
-def test_add_bad_user_3():
-    # Check user already exist
+def test_create_user_already_exists():
     with pytest.raises(MoulinetteError):
         user_create("alice", "Alice", "White", "other@" + maindomain, "test123Ynh")
 
-def test_del_bad_user_1():
-    # Check user not found
+def test_del_user_that_does_not_exist():
     with pytest.raises(MoulinetteError):
-        user_delete("not_exit")
+        user_delete("doesnt_exist")
 
-def test_add_bad_group_1():
+def test_create_group_all_users():
     # Check groups already exist with special group "all_users"
     with pytest.raises(YunohostError):
         user_group_create("all_users")
 
-def test_add_bad_group_2():
-    # Check groups already exist (for standard groups)
+def test_create_group_already_exists():
+    # Check groups already exist (regular groups)
     with pytest.raises(MoulinetteError):
         user_group_create("dev")
 
-def test_del_bad_group_1():
-    # Check not allowed to remove this groups
+def test_del_group_all_users():
     with pytest.raises(YunohostError):
         user_group_delete("all_users")
 
-def test_del_bad_group_2():
-    # Check groups not found
+def test_del_group_that_does_not_exist():
     with pytest.raises(MoulinetteError):
-        user_group_delete("not_exit")
+        user_group_delete("doesnt_exist")
 
 #
 # Update function
 #
 
-def test_update_user_1():
+def test_update_user():
     user_update("alice", firstname="NewName", lastname="NewLast")
 
     info = user_info("alice")
-    assert "NewName" == info['firstname']
-    assert "NewLast" == info['lastname']
+    assert info['firstname'] == "NewName"
+    assert info['lastname'] == "NewLast"
 
-def test_update_group_1():
+def test_update_group_add_user():
     user_group_update("dev", add=["bob"])
 
     group_res = user_group_list()['groups']
-    assert set(["alice", "bob"]) == set(group_res['dev']['members'])
+    assert set(group_res['dev']['members']) == set(["alice", "bob"])
 
-def test_update_group_2():
-    # Try to add a user in a group when the user is already in
+def test_update_group_add_user_already_in():
     user_group_update("apps", add=["bob"])
 
     group_res = user_group_list()['groups']
-    assert ["bob"] == group_res['apps']['members']
+    assert group_res['apps']['members'] == ["bob"]
 
-def test_update_group_3():
-    # Try to remove a user in a group
+def test_update_group_remove_user():
     user_group_update("apps", remove=["bob"])
 
     group_res = user_group_list()['groups']
-    assert "members" not in group_res['apps']
+    assert group_res['apps']['members'] == []
 
-def test_update_group_4():
-    # Try to remove a user in a group when it is not already in
+def test_update_group_remove_user_not_already_in():
     user_group_update("apps", remove=["jack"])
 
     group_res = user_group_list()['groups']
-    assert ["bob"] == group_res['apps']['members']
+    assert group_res['apps']['members'] == ["bob"]
 
 #
 # Error on update functions
 #
 
-def test_bad_update_user_1():
-    # Check user not found
+def test_update_user_that_doesnt_exist():
     with pytest.raises(YunohostError):
-        user_update("not_exit", firstname="NewName", lastname="NewLast")
+        user_update("doesnt_exist", firstname="NewName", lastname="NewLast")
 
-
-def bad_update_group_1():
+def test_update_group_that_doesnt_exist():
     # Check groups not found
     with pytest.raises(YunohostError):
-        user_group_update("not_exit", add=["alice"])
+        user_group_update("doesnt_exist", add=["alice"])
 
-def test_bad_update_group_2():
-    # Check remove user in groups "all_users" not allowed
+def test_update_group_all_users_manually():
     with pytest.raises(YunohostError):
         user_group_update("all_users", remove=["alice"])
 
-def test_bad_update_group_3():
-    # Check remove user in it own group not allowed
+    assert "alice" in user_group_list()["groups"]["all_users"]["members"]
+
+def test_update_group_primary_manually():
     with pytest.raises(YunohostError):
         user_group_update("alice", remove=["alice"])
+    assert "alice" in user_group_list()["groups"]["alice"]["members"]
 
-def test_bad_update_group_1():
+def test_update_group_add_user_that_doesnt_exist():
     # Check add bad user in group
     with pytest.raises(YunohostError):
-        user_group_update("dev", add=["not_exist"])
+        user_group_update("dev", add=["doesnt_exist"])
 
-    assert "not_exist" not in user_group_list()["groups"]["dev"]
+    assert "doesnt_exist" not in user_group_list()["groups"]["dev"]["members"]

From 68db93cd635a8ce5720f3e8b1ca105e6fa4a27d4 Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Thu, 12 Sep 2019 04:03:04 +0200
Subject: [PATCH 26/45] Fix an issue about groups not being properly cleaned
 and perms synced when deleting a user

---
 src/yunohost/user.py | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/src/yunohost/user.py b/src/yunohost/user.py
index e1719d3a6..ef2a7d523 100644
--- a/src/yunohost/user.py
+++ b/src/yunohost/user.py
@@ -245,9 +245,18 @@ def user_delete(operation_logger, username, purge=False):
     """
     from yunohost.hook import hook_callback
     from yunohost.utils.ldap import _get_ldap_interface
+    from yunohost.permission import permission_sync_to_user
 
     operation_logger.start()
 
+    user_group_update("all_users", remove=username, force=True, sync_perm=False)
+    for group, infos in user_group_list()["groups"].items():
+        # If the user is in this group (and it's not the primary group),
+        # remove the member from the group
+        if username != group and username in infos["members"]:
+            user_group_update(group, remove=username, sync_perm=False)
+    user_group_delete(username, force=True, sync_perm=True)
+
     ldap = _get_ldap_interface()
     if ldap.remove('uid=%s,ou=users' % username):
         # Invalidate passwd to take user deletion into account
@@ -259,19 +268,6 @@ def user_delete(operation_logger, username, purge=False):
     else:
         raise YunohostError('user_deletion_failed')
 
-    user_group_delete(username, force=True, sync_perm=True)
-
-    group_list = ldap.search('ou=groups,dc=yunohost,dc=org',
-                             '(&(objectclass=groupOfNamesYnh)(memberUid=%s))'
-                             % username, ['cn'])
-    for group in group_list:
-        user_list = ldap.search('ou=groups,dc=yunohost,dc=org',
-                                'cn=' + group['cn'][0],
-                                ['memberUid'])[0]
-        user_list['memberUid'].remove(username)
-        if not ldap.update('cn=%s,ou=groups' % group['cn'][0], user_list):
-            raise YunohostError('group_update_failed')
-
     hook_callback('post_user_delete', args=[username, purge])
 
     logger.success(m18n.n('user_deleted'))

From fe8f7f2210d9cc398dc27f9fa16b20cc11bd79b9 Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Thu, 12 Sep 2019 17:49:14 +0200
Subject: [PATCH 27/45] Update permission helper : have a single helper to
 manage urls, and a helper to add/remove groups to permission

---
 data/helpers.d/setting | 49 +++++++++++++++++++++++++++++-------------
 1 file changed, 34 insertions(+), 15 deletions(-)

diff --git a/data/helpers.d/setting b/data/helpers.d/setting
index 0e432d916..502da1ed7 100644
--- a/data/helpers.d/setting
+++ b/data/helpers.d/setting
@@ -261,32 +261,51 @@ ynh_permission_delete() {
     yunohost tools shell -c "from yunohost.permission import permission_delete; permission_delete('$app.$permission', sync_perm=False)"
 }
 
-# Add a path managed by the SSO
+# Manage urls related to a permission
 #
-# usage: ynh_permission_add_url --permission "permission" --url "url" ["url" ...]
+# usage: ynh_permission_urls --permission "permission" --add "url" ["url" ...] --remove "url" ["url" ...]
 # | arg: permission - the name for the permission (by default a permission named "main" is removed automatically when the app is removed)
-# | arg: urls - (optional) a list of FULL urls for the permission (e.g. domain.tld/apps/admin)
+# | arg: add - (optional) a list of FULL urls to add to the permission (e.g. domain.tld/apps/admin)
+# | arg: remove - (optional) a list of FULL urls to remove from the permission (e.g. other.tld/apps/admin)
 #
-ynh_permission_add_url() {
-    declare -Ar args_array=([p]=permission= [u]=urls= )
+ynh_permission_urls() {
+    declare -Ar args_array=([p]=permission= [a]=add= [r]=remove=)
     local permission
-    local urls
+    local add
+    local remove
     ynh_handle_getopts_args "$@"
 
-    yunohost tools shell -c "from yunohost.permission import permission_urls; permission_urls('$app.$permission', add=['${urls//';'/"','"}'], sync_perm=False)"
+    if [[ -n ${add:-} ]]; then
+        add=",add=['${add//';'/"','"}']"
+    fi
+    if [[ -n ${remove:-} ]]; then
+        remove=",remove=['${remove//';'/"','"}']"
+    fi
+
+    yunohost tools shell -c "from yunohost.permission import permission_urls; permission_urls('$app.$permission' ${add:-} ${remove:-})"
 }
 
-# Remove a path managed by the SSO
+# Update a permission for the app
 #
-# usage: ynh_permission_del_path --app "app" --permission "permission" --url "url" ["url" ...]
-# | arg: permission - the name for the permission (by default a permission named "main" is removed automatically when the app is removed)
-# | arg: urls - (optional) a list of FULL urls for the permission (e.g. domain.tld/apps/admin)
+# usage: ynh_permission_update --permission "permission" --add "group" ["group" ...] --remove  "group" ["group" ...]
+# | arg: permission - the name for the permission (by default a permission named "main" already exist)
+# | arg: add - the list of group or users to enable add to the permission
+# | arg: remove - the list of group or users to remove from the permission
 #
-ynh_permission_remove_url() {
-    declare -Ar args_array=([p]=permission= [u]=urls= )
+# example: ynh_permission_update --permission admin --add samdoe --remove all_users
+ynh_permission_update() {
+    declare -Ar args_array=( [p]=permission= [a]=add= [r]=remove= )
     local permission
-    local urls
+    local add
+    local remove
     ynh_handle_getopts_args "$@"
 
-    yunohost tools shell -c "from yunohost.permission import permission_urls; permission_urls('$app.$permission', remove=['${url//';'/"','"}'], sync_perm=False)"
+    if [[ -n ${add:-} ]]; then
+        add="--add ${add//';'/" "}"
+    fi
+    if [[ -n ${remove:-} ]]; then
+        remove="--remove ${remove//';'/" "} "
+    fi
+
+    yunohost user permission update "$app.$permission" ${add:-} ${remove:-}
 }

From b912cd0aecd48ec36d87d611447c91c97d293939 Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Thu, 12 Sep 2019 17:49:35 +0200
Subject: [PATCH 28/45] Propagate all changes to tests

---
 src/yunohost/tests/test_permission.py | 353 +++++++++++---------------
 1 file changed, 153 insertions(+), 200 deletions(-)

diff --git a/src/yunohost/tests/test_permission.py b/src/yunohost/tests/test_permission.py
index 0373a6cbf..2df6362a9 100644
--- a/src/yunohost/tests/test_permission.py
+++ b/src/yunohost/tests/test_permission.py
@@ -5,7 +5,8 @@ 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, \
                           user_group_list, user_group_create, user_group_delete, user_group_update, user_group_info
-from yunohost.permission import user_permission_update, user_permission_list, permission_create, permission_urls, permission_delete
+from yunohost.permission import user_permission_update, user_permission_list, user_permission_reset, \
+                                permission_create, permission_urls, permission_delete
 from yunohost.domain import _get_maindomain
 from yunohost.utils.error import YunohostError
 
@@ -20,20 +21,18 @@ def clean_user_groups_permission():
         if g != "all_users":
             user_group_delete(g)
 
-    for a, per in user_permission_list()['permissions'].items():
-        if a in ['wiki', 'blog', 'site']:
-            for p in per:
-                permission_delete(a, p, force=True, sync_perm=False)
+    for p in user_permission_list()['permissions']:
+        if any(p.startswith(name) for name in ["wiki", "blog", "site", "permissions_app"]):
+            permission_delete(p, force=True, sync_perm=False)
 
 def setup_function(function):
     clean_user_groups_permission()
 
     user_create("alice", "Alice", "White", "alice@" + maindomain, "test123Ynh")
     user_create("bob", "Bob", "Snow", "bob@" + maindomain, "test123Ynh")
-    permission_create("wiki", "main", [maindomain + "/wiki"], sync_perm=False)
-    permission_create("blog", "main", sync_perm=False)
-
-    user_permission_add(["blog"], "main", group="alice")
+    permission_create("wiki.main", urls=[maindomain + "/wiki"], sync_perm=False)
+    permission_create("blog.main", sync_perm=False)
+    user_permission_update("blog.main", remove="all_users", add="alice")
 
 def teardown_function(function):
     clean_user_groups_permission()
@@ -144,100 +143,89 @@ def check_permission_for_apps():
     # and we don't have any permission linked to no apps. The only exception who is not liked to an app
     # is mail, xmpp, and sftp
 
-    from yunohost.utils.ldap import _get_ldap_interface
-    ldap = _get_ldap_interface()
-    permission_search = ldap.search('ou=permission,dc=yunohost,dc=org',
-                                    '(objectclass=permissionYnh)',
-                                    ['cn', 'groupPermission', 'inheritPermission', 'memberUid'])
+    app_perms = user_permission_list(ignore_system_perms=True)["permissions"].keys()
+
+    # Keep only the prefix so that
+    # ["foo.main", "foo.pwet", "bar.main"]
+    # becomes
+    # {"bar", "foo"}
+    # and compare this to the list of installed apps ...
+
+    app_perms_prefix = set(p.split(".")[0] for p in app_perms)
 
     installed_apps = {app['id'] for app in app_list(installed=True)['apps']}
-    permission_list_set = {permission['cn'][0].split(".")[1] for permission in permission_search}
 
-    extra_service_permission = set(['mail', 'xmpp'])
-    if 'sftp' in permission_list_set:
-        extra_service_permission.add('sftp')
-    assert installed_apps == permission_list_set - extra_service_permission
+    assert installed_apps == app_perms_prefix
 
 #
 # List functions
 #
 
-def test_list_permission():
-    res = user_permission_list()['permissions']
+def test_permission_list():
+    res = user_permission_list(full=True)['permissions']
 
-    assert "wiki" in res
-    assert "main" in res['wiki']
-    assert "blog" in res
-    assert "main" in res['blog']
-    assert "mail" in res
-    assert "main" in res['mail']
-    assert "xmpp" in res
-    assert "main" in res['xmpp']
-    assert ["all_users"] == res['wiki']['main']['allowed_groups']
-    assert ["alice"] == res['blog']['main']['allowed_groups']
-    assert set(["alice", "bob"]) == set(res['wiki']['main']['allowed_users'])
-    assert ["alice"] == res['blog']['main']['allowed_users']
-    assert [maindomain + "/wiki"] == res['wiki']['main']['URL']
+    assert "wiki.main" in res
+    assert "blog.main" in res
+    assert "mail.main" in res
+    assert "xmpp.main" in res
+    assert res['wiki.main']['allowed'] == ["all_users"]
+    assert res['blog.main']['allowed'] == ["alice"]
+    assert set(res['wiki.main']['corresponding_users']) == set(["alice", "bob"])
+    assert res['blog.main']['corresponding_users'] == ["alice"]
+    assert res['wiki.main']['urls'] == [maindomain + "/wiki"]
 
 #
 # Create - Remove functions
 #
 
-def test_add_permission_1():
-    permission_create("site", "test")
+def test_permission_create_main():
+    permission_create("site.main")
+
+    res = user_permission_list(full=True)['permissions']
+    assert "site.main" in res
+    assert res['site.main']['allowed'] == ["all_users"]
+    assert set(res['site.main']['corresponding_users']) == set(["alice", "bob"])
+
+
+def test_permission_create_extra():
+    permission_create("site.test")
+
+    res = user_permission_list(full=True)['permissions']
+    assert "site.test" in res
+    # all_users is only enabled by default on .main perms
+    assert "all_users" not in res['site.test']['allowed']
+    assert res['site.test']['corresponding_users'] == []
+
+def test_permission_delete():
+    permission_delete("wiki.main", force=True)
 
     res = user_permission_list()['permissions']
-    assert "site" in res
-    assert "test" in res['site']
-    assert "all_users" in res['site']['test']['allowed_groups']
-    assert set(["alice", "bob"]) == set(res['site']['test']['allowed_users'])
-
-def test_add_permission_2():
-    permission_create("site", "main", default_allow=False)
-
-    res = user_permission_list()['permissions']
-    assert "site" in res
-    assert "main" in res['site']
-    assert [] == res['site']['main']['allowed_groups']
-    assert [] == res['site']['main']['allowed_users']
-
-def test_remove_permission():
-    permission_delete("wiki", "main", force=True)
-
-    res = user_permission_list()['permissions']
-    assert "wiki" not in res
+    assert "wiki.main" not in res
 
 #
 # Error on create - remove function
 #
 
-def test_add_bad_permission():
-    # Create permission with same name
+def test_permission_create_already_existing():
     with pytest.raises(YunohostError):
-        permission_create("wiki", "main")
+        permission_create("wiki.main")
 
-def test_remove_bad_permission():
-    # Remove not existant permission
+def test_permission_delete_doesnt_existing():
     with pytest.raises(MoulinetteError):
-        permission_delete("non_exit", "main", force=True)
+        permission_delete("doesnt.exist", force=True)
 
     res = user_permission_list()['permissions']
-    assert "wiki" in res
-    assert "main" in res['wiki']
-    assert "blog" in res
-    assert "main" in res['blog']
-    assert "mail" in res
-    assert "main" in res['mail']
-    assert "xmpp" in res
-    assert "main" in res['xmpp']
+    assert "wiki.main" in res
+    assert "blog.main" in res
+    assert "mail.main" in res
+    assert "xmpp.main" in res
 
-def test_remove_main_permission():
+def test_permission_delete_main_without_force():
     with pytest.raises(YunohostError):
-        permission_delete("blog", "main")
+        permission_delete("blog.main")
 
     res = user_permission_list()['permissions']
-    assert "mail" in res
-    assert "main" in res['mail']
+    assert "blog.main" in res
 
 #
 # Update functions
@@ -245,137 +233,100 @@ def test_remove_main_permission():
 
 # user side functions
 
-def test_allow_first_group():
-    # Remove permission to all_users and define per users
-    user_permission_add(["wiki"], "main", group="alice")
+def test_permission_add_group():
+    user_permission_update("wiki.main", add="alice")
 
-    res = user_permission_list()['permissions']
-    assert ['alice'] == res['wiki']['main']['allowed_users']
-    assert ['alice'] == res['wiki']['main']['allowed_groups']
+    res = user_permission_list(full=True)['permissions']
+    assert set(res['wiki.main']['allowed']) == set(["all_users", "alice"])
+    assert set(res['wiki.main']['corresponding_users']) == set(["alice", "bob"])
 
-def test_allow_other_group():
-    # Allow new user in a permission
-    user_permission_add(["blog"], "main", group="bob")
+def test_permission_remove_group():
+    user_permission_update("blog.main", remove="alice")
 
-    res = user_permission_list()['permissions']
-    assert set(["alice", "bob"]) == set(res['blog']['main']['allowed_users'])
-    assert set(["alice", "bob"]) == set(res['blog']['main']['allowed_groups'])
+    res = user_permission_list(full=True)['permissions']
+    assert res['blog.main']['allowed'] == []
+    assert res['blog.main']['corresponding_users'] == []
 
-def test_disallow_group_1():
-    # Disallow a user in a permission
-    user_permission_remove(["blog"], "main", group="alice")
+def test_permission_add_and_remove_group():
+    user_permission_update("wiki.main", add="alice", remove="all_users")
 
-    res = user_permission_list()['permissions']
-    assert [] == res['blog']['main']['allowed_users']
-    assert [] == res['blog']['main']['allowed_groups']
+    res = user_permission_list(full=True)['permissions']
+    assert res['wiki.main']['allowed'] == ["alice"]
+    assert res['wiki.main']['corresponding_users'] == ["alice"]
 
-def test_allow_group_1():
-    # Allow a user when he is already allowed
-    user_permission_add(["blog"], "main", group="alice")
+def test_permission_add_group_already_allowed():
+    user_permission_update("blog.main", add="alice")
 
-    res = user_permission_list()['permissions']
-    assert ["alice"] == res['blog']['main']['allowed_users']
-    assert ["alice"] == res['blog']['main']['allowed_groups']
+    res = user_permission_list(full=True)['permissions']
+    assert res['blog.main']['allowed'] == ["alice"]
+    assert res['blog.main']['corresponding_users'] == ["alice"]
 
-def test_disallow_group_1():
-    # Disallow a user when he is already disallowed
-    user_permission_remove(["blog"], "main", group="bob")
+def test_permission_remove_group_already_not_allowed():
+    user_permission_update("blog.main", remove="bob")
 
-    res = user_permission_list()['permissions']
-    assert ["alice"] == res['blog']['main']['allowed_users']
-    assert ["alice"] == res['blog']['main']['allowed_groups']
+    res = user_permission_list(full=True)['permissions']
+    assert res['blog.main']['allowed'] == ["alice"]
+    assert res['blog.main']['corresponding_users'] == ["alice"]
 
-def test_reset_permission():
+def test_permission_reset():
     # Reset permission
-    user_permission_clear(["blog"], "main")
+    user_permission_reset("blog.main")
 
-    res = user_permission_list()['permissions']
-    assert set(["alice", "bob"]) == set(res['blog']['main']['allowed_users'])
-    assert ["all_users"] == res['blog']['main']['allowed_groups']
-
-# internal functions
-
-def test_add_url_1():
-    # Add URL in permission which hasn't any URL defined
-    permission_update("blog", "main", add_url=[maindomain + "/testA"])
-
-    res = user_permission_list()['permissions']
-    assert [maindomain + "/testA"] == res['blog']['main']['URL']
-
-def test_add_url_2():
-    # Add a second URL in a permission
-    permission_update("wiki", "main", add_url=[maindomain + "/testA"])
-
-    res = user_permission_list()['permissions']
-    assert set([maindomain + "/testA", maindomain + "/wiki"]) == set(res['wiki']['main']['URL'])
-
-def test_remove_url_1():
-    permission_update("wiki", "main", remove_url=[maindomain + "/wiki"])
-
-    res = user_permission_list()['permissions']
-    assert 'URL' not in res['wiki']['main']
-
-def test_add_url_3():
-    # Add a url already added
-    permission_update("wiki", "main", add_url=[maindomain + "/wiki"])
-
-    res = user_permission_list()['permissions']
-    assert [maindomain + "/wiki"] == res['wiki']['main']['URL']
-
-def test_remove_url_2():
-    # Remove a url not added (with a permission which contain some URL)
-    permission_update("wiki", "main", remove_url=[maindomain + "/not_exist"])
-
-    res = user_permission_list()['permissions']
-    assert [maindomain + "/wiki"] == res['wiki']['main']['URL']
-
-def test_remove_url_2():
-    # Remove a url not added (with a permission which contain no URL)
-    permission_update("blog", "main", remove_url=[maindomain + "/not_exist"])
-
-    res = user_permission_list()['permissions']
-    assert 'URL' not in res['blog']['main']
+    res = user_permission_list(full=True)['permissions']
+    assert res['blog.main']['allowed'] == ["all_users"]
+    assert set(res['blog.main']['corresponding_users']) == set(["alice", "bob"])
 
 #
 # Error on update function
 #
 
-def test_disallow_bad_group_1():
-    # Disallow a group when the group all_users is allowed
+def test_permission_add_group_that_doesnt_exist():
     with pytest.raises(YunohostError):
-        user_permission_remove("wiki", "main", group="alice")
+        user_permission_update("blog.main", add="doesnt_exist")
 
-    res = user_permission_list()['permissions']
-    assert ["all_users"] == res['wiki']['main']['allowed_groups']
-    assert set(["alice", "bob"]) == set(res['wiki']['main']['allowed_users'])
+    res = user_permission_list(full=True)['permissions']
+    assert res['blog.main']['allowed'] == ["alice"]
+    assert res['blog.main']['corresponding_users'] == ["alice"]
 
-def test_allow_bad_user():
-    # Allow a non existant group
+def test_permission_update_permission_that_doesnt_exist():
     with pytest.raises(YunohostError):
-        user_permission_add(["blog"], "main", group="not_exist")
+        user_permission_update("doesnt.exist", add="alice")
 
-    res = user_permission_list()['permissions']
-    assert ["alice"] == res['blog']['main']['allowed_groups']
-    assert ["alice"] == res['blog']['main']['allowed_users']
 
-def test_disallow_bad_group_2():
-    # Disallow a non existant group
-    with pytest.raises(YunohostError):
-        user_permission_remove(["blog"], "main", group="not_exist")
+# Permission url management
 
-    res = user_permission_list()['permissions']
-    assert ["alice"] == res['blog']['main']['allowed_groups']
-    assert ["alice"] == res['blog']['main']['allowed_users']
+def test_permission_add_url():
+    permission_urls("blog.main", add=[maindomain + "/testA"])
 
-def test_allow_bad_permission_1():
-    # Allow a user to a non existant permission
-    with pytest.raises(YunohostError):
-        user_permission_add(["wiki"], "not_exit", group="alice")
+    res = user_permission_list(full=True)['permissions']
+    assert res["blog.main"]["urls"] == [maindomain + "/testA"]
 
-def test_allow_bad_permission_2():
-    # Allow a user to a non existant permission
-    with pytest.raises(YunohostError):
-        user_permission_add(["not_exit"], "main", group="alice")
+def test_permission_add_second_url():
+    permission_urls("wiki.main", add=[maindomain + "/testA"])
+
+    res = user_permission_list(full=True)['permissions']
+    assert set(res["wiki.main"]["urls"]) == set([maindomain + "/testA", maindomain + "/wiki"])
+
+def test_permission_remove_url():
+    permission_urls("wiki.main", remove=[maindomain + "/wiki"])
+
+    res = user_permission_list(full=True)['permissions']
+    assert res["wiki.main"]["urls"] == []
+
+def test_permission_add_url_already_added():
+    res = user_permission_list(full=True)['permissions']
+    assert res["wiki.main"]["urls"] == [maindomain + "/wiki"]
+
+    permission_urls("wiki.main", add=[maindomain + "/wiki"])
+
+    res = user_permission_list(full=True)['permissions']
+    assert res["wiki.main"]["urls"] == [maindomain + "/wiki"]
+
+def test_permission_remove_url_not_added():
+    permission_urls("wiki.main", remove=[maindomain + "/doesnt_exist"])
+
+    res = user_permission_list(full=True)['permissions']
+    assert res['wiki.main']['urls'] == [maindomain + "/wiki"]
 
 #
 # Application interaction
@@ -385,42 +336,44 @@ def test_install_app():
     app_install("./tests/apps/permissions_app_ynh",
                 args="domain=%s&path=%s&admin=%s" % (maindomain, "/urlpermissionapp", "alice"), force=True)
 
-    res = user_permission_list()['permissions']
-    assert "permissions_app" in res
-    assert "main" in res['permissions_app']
-    assert [maindomain + "/urlpermissionapp"] == res['permissions_app']['main']['URL']
-    assert [maindomain + "/urlpermissionapp/admin"] == res['permissions_app']['admin']['URL']
-    assert [maindomain + "/urlpermissionapp/dev"] == res['permissions_app']['dev']['URL']
+    res = user_permission_list(full=True)['permissions']
+    assert "permissions_app.main" in res
+    assert "permissions_app.admin" in res
+    assert "permissions_app.dev" in res
+    assert res['permissions_app.main']['urls'] == [maindomain + "/urlpermissionapp"]
+    assert res['permissions_app.admin']['urls'] == [maindomain + "/urlpermissionapp/admin"]
+    assert res['permissions_app.dev']['urls'] == [maindomain + "/urlpermissionapp/dev"]
 
-    assert ["all_users"] == res['permissions_app']['main']['allowed_groups']
-    assert set(["alice", "bob"]) == set(res['permissions_app']['main']['allowed_users'])
+    assert res['permissions_app.main']['allowed'] == ["all_users"]
+    assert set(res['permissions_app.main']['corresponding_users']) == set(["alice", "bob"])
 
-    assert ["alice"] == res['permissions_app']['admin']['allowed_groups']
-    assert ["alice"] == res['permissions_app']['admin']['allowed_users']
+    assert res['permissions_app.admin']['allowed'] == ["alice"]
+    assert res['permissions_app.admin']['corresponding_users'] == ["alice"]
 
-    assert ["all_users"] == res['permissions_app']['dev']['allowed_groups']
-    assert set(["alice", "bob"]) == set(res['permissions_app']['dev']['allowed_users'])
+    assert res['permissions_app.dev']['allowed'] == []
+    assert set(res['permissions_app.dev']['corresponding_users']) == set()
 
 def test_remove_app():
     app_install("./tests/apps/permissions_app_ynh",
                 args="domain=%s&path=%s&admin=%s" % (maindomain, "/urlpermissionapp", "alice"), force=True)
     app_remove("permissions_app")
 
-    res = user_permission_list()['permissions']
-    assert "permissions_app" not in res
+    # Check all permissions for this app got deleted
+    res = user_permission_list(full=True)['permissions']
+    assert not any(p.startswith("permissions_app.") for p in res.keys())
 
 def test_change_url():
     app_install("./tests/apps/permissions_app_ynh",
                 args="domain=%s&path=%s&admin=%s" % (maindomain, "/urlpermissionapp", "alice"), force=True)
 
-    res = user_permission_list()['permissions']
-    assert [maindomain + "/urlpermissionapp"] == res['permissions_app']['main']['URL']
-    assert [maindomain + "/urlpermissionapp/admin"] == res['permissions_app']['admin']['URL']
-    assert [maindomain + "/urlpermissionapp/dev"] == res['permissions_app']['dev']['URL']
+    res = user_permission_list(full=True)['permissions']
+    assert res['permissions_app.main']['urls'] == [maindomain + "/urlpermissionapp"]
+    assert res['permissions_app.admin']['urls'] == [maindomain + "/urlpermissionapp/admin"]
+    assert res['permissions_app.dev']['urls'] == [maindomain + "/urlpermissionapp/dev"]
 
     app_change_url("permissions_app", maindomain, "/newchangeurl")
 
-    res = user_permission_list()['permissions']
-    assert [maindomain + "/newchangeurl"] == res['permissions_app']['main']['URL']
-    assert [maindomain + "/newchangeurl/admin"] == res['permissions_app']['admin']['URL']
-    assert [maindomain + "/newchangeurl/dev"] == res['permissions_app']['dev']['URL']
+    res = user_permission_list(full=True)['permissions']
+    assert res['permissions_app.main']['urls'] == [maindomain + "/newchangeurl"]
+    assert res['permissions_app.admin']['urls'] == [maindomain + "/newchangeurl/admin"]
+    assert res['permissions_app.dev']['urls'] == [maindomain + "/newchangeurl/dev"]

From 2e14834e6b98ed29277c57d8f2e4ffce88b04cd8 Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Thu, 12 Sep 2019 17:50:52 +0200
Subject: [PATCH 29/45] Misc fixes following tests

---
 locales/en.json            | 10 +++++-----
 src/yunohost/app.py        |  6 ++----
 src/yunohost/permission.py |  6 +++---
 src/yunohost/user.py       |  2 +-
 4 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/locales/en.json b/locales/en.json
index c370f821e..7a16ebd0c 100644
--- a/locales/en.json
+++ b/locales/en.json
@@ -274,9 +274,9 @@
     "log_dyndns_subscribe": "Subscribe to a YunoHost subdomain '{}'",
     "log_dyndns_update": "Update the ip associated with your YunoHost subdomain '{}'",
     "log_letsencrypt_cert_install": "Install Let's encrypt certificate on '{}' domain",
-    "log_permission_create": "Create permission '{permission}'",
-    "log_permission_delete": "Delete permission '{permission}'",
-    "log_permission_urls": "Update urls related to permission '{permission}'",
+    "log_permission_create": "Create permission '{}'",
+    "log_permission_delete": "Delete permission '{}'",
+    "log_permission_urls": "Update urls related to permission '{}'",
     "log_selfsigned_cert_install": "Install self signed certificate on '{}' domain",
     "log_letsencrypt_cert_renew": "Renew '{}' Let's encrypt certificate",
     "log_regen_conf": "Regenerate system configurations '{}'",
@@ -286,8 +286,8 @@
     "log_user_group_delete": "Delete '{}' group",
     "log_user_group_update": "Update '{}' group",
     "log_user_update": "Update information of '{}' user",
-    "log_user_permission_update": "Update accesses for permission '{permission}'",
-    "log_user_permission_reset": "Reset permission '{permission}'",
+    "log_user_permission_update": "Update accesses for permission '{}'",
+    "log_user_permission_reset": "Reset permission '{}'",
     "log_tools_maindomain": "Make '{}' as main domain",
     "log_tools_migrations_migrate_forward": "Migrate forward",
     "log_tools_postinstall": "Postinstall your YunoHost server",
diff --git a/src/yunohost/app.py b/src/yunohost/app.py
index f505dd088..b3c36d059 100644
--- a/src/yunohost/app.py
+++ b/src/yunohost/app.py
@@ -735,11 +735,9 @@ def app_install(operation_logger, app, label=None, args=None, no_remove_on_failu
     if packages.dpkg_is_broken():
         raise YunohostError("dpkg_is_broken")
 
-    from yunohost.utils.ldap import _get_ldap_interface
     from yunohost.hook import hook_add, hook_remove, hook_exec, hook_callback
     from yunohost.log import OperationLogger
-    from yunohost.permission import permission_create, permission_urls, permission_delete, permission_sync_to_user
-    ldap = _get_ldap_interface()
+    from yunohost.permission import user_permission_list, permission_create, permission_urls, permission_delete, permission_sync_to_user
 
     # Fetch or extract sources
     if not os.path.exists(INSTALL_TMP):
@@ -976,7 +974,7 @@ def app_remove(operation_logger, app):
 
     """
     from yunohost.hook import hook_exec, hook_remove, hook_callback
-    from yunohost.permission import permission_delete, permission_sync_to_user
+    from yunohost.permission import user_permission_list, permission_delete, permission_sync_to_user
     if not _is_installed(app):
         raise YunohostError('app_not_installed', app=app, all_apps=_get_all_installed_apps_id())
 
diff --git a/src/yunohost/permission.py b/src/yunohost/permission.py
index 4d935d3c0..e5035b0ad 100644
--- a/src/yunohost/permission.py
+++ b/src/yunohost/permission.py
@@ -45,7 +45,7 @@ SYSTEM_PERMS = ["mail", "xmpp", "stfp"]
 #
 
 
-def user_permission_list(short=False, full=False, ignore_system_perms=True):
+def user_permission_list(short=False, full=False, ignore_system_perms=False):
     """
     List permissions and corresponding accesses
     """
@@ -273,13 +273,13 @@ def permission_create(operation_logger, permission, urls=None, sync_perm=True):
 
     attr_dict = {
         'objectClass': ['top', 'permissionYnh', 'posixGroup'],
-        'cn': permission,
+        'cn': str(permission),
         'gidNumber': gid,
     }
 
     # For main permission, we add all users by default
     if permission.endswith(".main"):
-        attr_dict['groupPermission'] = 'cn=all_users,ou=groups,dc=yunohost,dc=org'
+        attr_dict['groupPermission'] = ['cn=all_users,ou=groups,dc=yunohost,dc=org']
 
     if urls:
         attr_dict['URL'] = [_normalize_url(url) for url in urls]
diff --git a/src/yunohost/user.py b/src/yunohost/user.py
index ef2a7d523..bb4d6aed2 100644
--- a/src/yunohost/user.py
+++ b/src/yunohost/user.py
@@ -449,7 +449,7 @@ def user_info(username):
 
         if service_status("dovecot")["status"] != "running":
             logger.warning(m18n.n('mailbox_used_space_dovecot_down'))
-        elif username not in user_permission_list()["permissions"]["mail.main"]["allowed_users"]:
+        elif username not in user_permission_list(full=True)["permissions"]["mail.main"]["corresponding_users"]:
             logger.warning(m18n.n('mailbox_disabled', user=username))
         else:
             cmd = 'doveadm -f flow quota get -u %s' % user['uid'][0]

From bdad4ffd7114602aea0303793ebdc912da054905 Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Thu, 12 Sep 2019 18:34:17 +0200
Subject: [PATCH 30/45] c.f. issue 1405 ... those 'if ldap.stuff()' are
 complete bullshit from the very beginning since they never return False :
 instead they trigger an exception which means the current error management is
 completely meaningless ... so this refactorize all the places if found those
 + add proper error messages

---
 locales/en.json            |  22 ++++----
 src/yunohost/domain.py     |  16 ++++--
 src/yunohost/permission.py | 109 +++++++++++++++++++++----------------
 src/yunohost/user.py       | 109 ++++++++++++++++++++-----------------
 4 files changed, 144 insertions(+), 112 deletions(-)

diff --git a/locales/en.json b/locales/en.json
index 7a16ebd0c..d60a432ab 100644
--- a/locales/en.json
+++ b/locales/en.json
@@ -164,9 +164,9 @@
     "domain_cannot_remove_main": "Cannot remove main domain. Set a new main domain first",
     "domain_cert_gen_failed": "Unable to generate certificate",
     "domain_created": "The domain has been created",
-    "domain_creation_failed": "Unable to create domain",
+    "domain_creation_failed": "Failed to create domain {domain}: {error}",
     "domain_deleted": "The domain has been deleted",
-    "domain_deletion_failed": "Unable to delete domain",
+    "domain_deletion_failed": "Failed to delete domain {domain}: {error}",
     "domain_dns_conf_is_just_a_recommendation": "This command shows you what is the *recommended* configuration. It does not actually set up the DNS configuration for you. It is your responsability to configure your DNS zone in your registrar according to this recommendation.",
     "domain_dyndns_already_subscribed": "You've already subscribed to a DynDNS domain",
     "domain_dyndns_dynette_is_unreachable": "Unable to reach YunoHost dynette, either your YunoHost is not correctly connected to the internet or the dynette server is down. Error: {error}",
@@ -229,14 +229,14 @@
     "group_already_exist": "Group {group} already exist",
     "group_already_exist_on_system": "Group {group} already exists in the system group",
     "group_created": "Group '{group}' successfully created",
-    "group_creation_failed": "Group creation failed for group '{group}'",
+    "group_creation_failed": "Failed to create group {group}: {error}",
     "group_cannot_be_edited": "The group {group} cannot be edited manually.",
     "group_cannot_be_deleted": "The group {group} cannot be deleted manually.",
     "group_deleted": "Group '{group}' deleted",
-    "group_deletion_failed": "Group '{group} 'deletion failed",
+    "group_deletion_failed": "Failed to delete group {group}: {error}",
     "group_unknown": "Group {group} unknown",
     "group_updated": "Group '{group}' updated",
-    "group_update_failed": "Group update failed for group '{group}'",
+    "group_update_failed": "Failed to update group {group}: {error}",
     "group_user_already_in_group": "User {user} is already in group {group}",
     "group_user_not_in_group": "User {user} is not in group {group}",
     "hook_exec_failed": "Script execution failed: {path:s}",
@@ -430,11 +430,11 @@
     "permission_already_exist": "Permission '{permission}' already exists",
     "permission_cannot_remove_main": "Removing a main permission is not allowed",
     "permission_created": "Permission '{permission}' created",
-    "permission_creation_failed": "Failed to create permission '{permission}'",
+    "permission_creation_failed": "Failed to create permission '{permission}': {error}",
     "permission_deleted": "Permission '{permission}' deleted",
-    "permission_deletion_failed": "Failed to delete permission '{permission}'",
+    "permission_deletion_failed": "Failed to delete permission '{permission}': {error}",
     "permission_not_found": "Permission '{permission}' does not seem to exist ?",
-    "permission_update_failed": "Failed to update permission '{permission}'",
+    "permission_update_failed": "Failed to update permission '{permission}' : {error}",
     "permission_updated": "Permission '{permission}' updated",
     "permission_update_nothing_to_do": "No permissions to update",
     "port_already_closed": "Port {port:d} is already closed for {ip_version:s} connections",
@@ -556,13 +556,13 @@
     "upnp_enabled": "UPnP has been enabled",
     "upnp_port_open_failed": "Unable to open UPnP ports",
     "user_created": "The user has been created",
-    "user_creation_failed": "Unable to create user",
+    "user_creation_failed": "Unable to create user {user}: {error}",
     "user_deleted": "The user has been deleted",
-    "user_deletion_failed": "Unable to delete user",
+    "user_deletion_failed": "Unable to delete user {user}: {error}",
     "user_home_creation_failed": "Unable to create user home folder",
     "user_info_failed": "Unable to retrieve user information",
     "user_unknown": "Unknown user: {user:s}",
-    "user_update_failed": "Unable to update user",
+    "user_update_failed": "Unable to update user {user}: {error}",
     "user_updated": "The user has been updated",
     "users_available": "Available users:",
     "yunohost_already_installed": "YunoHost is already installed",
diff --git a/src/yunohost/domain.py b/src/yunohost/domain.py
index 42a4881ba..3f906748b 100644
--- a/src/yunohost/domain.py
+++ b/src/yunohost/domain.py
@@ -112,8 +112,10 @@ def domain_add(operation_logger, domain, dyndns=False):
             'virtualdomain': domain,
         }
 
-        if not ldap.add('virtualdomain=%s,ou=domains' % domain, attr_dict):
-            raise YunohostError('domain_creation_failed')
+        try:
+            ldap.add('virtualdomain=%s,ou=domains' % domain, attr_dict)
+        except Exception as e:
+            raise YunohostError('domain_creation_failed', domain=domain, error=e)
 
         # Don't regen these conf if we're still in postinstall
         if os.path.exists('/etc/yunohost/installed'):
@@ -167,10 +169,12 @@ def domain_remove(operation_logger, domain, force=False):
 
     operation_logger.start()
     ldap = _get_ldap_interface()
-    if ldap.remove('virtualdomain=' + domain + ',ou=domains') or force:
-        os.system('rm -rf /etc/yunohost/certs/%s' % domain)
-    else:
-        raise YunohostError('domain_deletion_failed')
+    try:
+        ldap.remove('virtualdomain=' + domain + ',ou=domains')
+    except Exception as e:
+        raise YunohostError('domain_deletion_failed', domain=domain, error=e)
+
+    os.system('rm -rf /etc/yunohost/certs/%s' % domain)
 
     regen_conf(names=['nginx', 'metronome', 'dnsmasq', 'postfix'])
     app_ssowatconf()
diff --git a/src/yunohost/permission.py b/src/yunohost/permission.py
index e5035b0ad..984a5d902 100644
--- a/src/yunohost/permission.py
+++ b/src/yunohost/permission.py
@@ -153,36 +153,37 @@ def user_permission_update(operation_logger, permission, add=None, remove=None,
 
     operation_logger.start()
 
-    if ldap.update('cn=%s,ou=permission' % permission,
-                     {'groupPermission': ['cn=' + g + ',ou=groups,dc=yunohost,dc=org' for g in new_allowed_groups]}):
-        logger.debug(m18n.n('permission_updated', permission=permission))
+    try:
+        ldap.update('cn=%s,ou=permission' % permission,
+                    {'groupPermission': ['cn=' + g + ',ou=groups,dc=yunohost,dc=org' for g in new_allowed_groups]})
+    except Exception as e:
+        raise YunohostError('permission_update_failed', permission=permission, error=e)
 
-        # Trigger permission sync if asked
+    logger.debug(m18n.n('permission_updated', permission=permission))
 
-        if sync_perm:
-            permission_sync_to_user()
+    # Trigger permission sync if asked
 
-        new_permission = user_permission_list(full=True)["permissions"][permission]
+    if sync_perm:
+        permission_sync_to_user()
 
-        # Trigger app callbacks
+    new_permission = user_permission_list(full=True)["permissions"][permission]
 
-        app = permission.split(".")[0]
+    # Trigger app callbacks
 
-        old_allowed_users = set(existing_permission["corresponding_users"])
-        new_allowed_users = set(new_permission["corresponding_users"])
+    app = permission.split(".")[0]
 
-        effectively_added_users = new_allowed_users - old_allowed_users
-        effectively_removed_users = old_allowed_users - new_allowed_users
+    old_allowed_users = set(existing_permission["corresponding_users"])
+    new_allowed_users = set(new_permission["corresponding_users"])
 
-        if effectively_added_users:
-            hook_callback('post_app_addaccess', args=[app, ','.join(effectively_added_users)])
-        if effectively_removed_users:
-            hook_callback('post_app_removeaccess', args=[app, ','.join(effectively_removed_users)])
+    effectively_added_users = new_allowed_users - old_allowed_users
+    effectively_removed_users = old_allowed_users - new_allowed_users
 
-        return new_permission
+    if effectively_added_users:
+        hook_callback('post_app_addaccess', args=[app, ','.join(effectively_added_users)])
+    if effectively_removed_users:
+        hook_callback('post_app_removeaccess', args=[app, ','.join(effectively_removed_users)])
 
-    else:
-        raise YunohostError('permission_update_failed', permission=permission)
+    return new_permission
 
 
 @is_unit_operation()
@@ -209,10 +210,12 @@ def user_permission_reset(operation_logger, permission, sync_perm=True):
     operation_logger.start()
 
     default_permission = {'groupPermission': ['cn=all_users,ou=groups,dc=yunohost,dc=org']}
-    if ldap.update('cn=%s,ou=permission' % permission, default_permission):
-        logger.debug(m18n.n('permission_updated', permission=permission))
-    else:
-        raise YunohostError('permission_update_failed', permission=permission)
+    try:
+        ldap.update('cn=%s,ou=permission' % permission, default_permission)
+    except Exception as e:
+        raise YunohostError('permission_update_failed', permission=permission, error=e)
+
+    logger.debug(m18n.n('permission_updated', permission=permission))
 
     if sync_perm:
         permission_sync_to_user()
@@ -286,13 +289,17 @@ def permission_create(operation_logger, permission, urls=None, sync_perm=True):
 
     operation_logger.related_to.append(('app', permission.split(".")[0]))
     operation_logger.start()
-    if ldap.add('cn=%s,ou=permission' % permission, attr_dict):
-        if sync_perm:
-            permission_sync_to_user()
-        logger.debug(m18n.n('permission_created', permission=permission))
-        return user_permission_list(full=True)["permissions"][permission]
-    else:
-        raise YunohostError('permission_creation_failed')
+
+    try:
+        ldap.add('cn=%s,ou=permission' % permission, attr_dict)
+    except Exception as e:
+        raise YunohostError('permission_creation_failed', permission=permission, error=e)
+
+    if sync_perm:
+        permission_sync_to_user()
+
+    logger.debug(m18n.n('permission_created', permission=permission))
+    return user_permission_list(full=True)["permissions"][permission]
 
 
 @is_unit_operation()
@@ -336,13 +343,17 @@ def permission_urls(operation_logger, permission, add=None, remove=None, sync_pe
 
     operation_logger.related_to.append(('app', permission.split(".")[0]))
     operation_logger.start()
-    if ldap.update('cn=%s,ou=permission' % permission, {'URL': new_urls}):
-        if sync_perm:
-            permission_sync_to_user()
-        logger.debug(m18n.n('permission_updated', permission=permission))
-        return user_permission_list(full=True)["permissions"][permission]
-    else:
-        raise YunohostError('permission_update_failed', permission=permission)
+
+    try:
+        ldap.update('cn=%s,ou=permission' % permission, {'URL': new_urls})
+    except Exception as e:
+        raise YunohostError('permission_update_failed', permission=permission, error=e)
+
+    if sync_perm:
+        permission_sync_to_user()
+
+    logger.debug(m18n.n('permission_updated', permission=permission))
+    return user_permission_list(full=True)["permissions"][permission]
 
 
 @is_unit_operation()
@@ -370,12 +381,15 @@ def permission_delete(operation_logger, permission, force=False, sync_perm=True)
 
     operation_logger.related_to.append(('app', permission.split(".")[0]))
     operation_logger.start()
-    if ldap.remove('cn=%s,ou=permission' % permission):
-        if sync_perm:
-            permission_sync_to_user()
-        logger.debug(m18n.n('permission_deleted', permission=permission))
-    else:
-        raise YunohostError('permission_deletion_failed', permission=permission)
+
+    try:
+        ldap.remove('cn=%s,ou=permission' % permission)
+    except Exception as e:
+        raise YunohostError('permission_deletion_failed', permission=permission, error=e)
+
+    if sync_perm:
+        permission_sync_to_user()
+    logger.debug(m18n.n('permission_deleted', permission=permission))
 
 
 def permission_sync_to_user():
@@ -410,8 +424,10 @@ def permission_sync_to_user():
                                'memberUid': should_be_allowed_users}
 
         # Commit the change with the new inherited stuff
-        if not ldap.update('cn=%s,ou=permission' % permission_name, new_inherited_perms):
-            raise YunohostError('permission_update_failed', permission=permission_name)
+        try:
+            ldap.update('cn=%s,ou=permission' % permission_name, new_inherited_perms)
+        except Exception as e:
+            raise YunohostError('permission_update_failed', permission=permission_name, error=e)
 
     logger.debug("The permission database has been resynchronized")
 
@@ -421,6 +437,7 @@ def permission_sync_to_user():
     os.system('nscd --invalidate=passwd')
     os.system('nscd --invalidate=group')
 
+
 def _normalize_url(url):
     from yunohost.domain import _normalize_domain_path
     domain = url[:url.index('/')]
diff --git a/src/yunohost/user.py b/src/yunohost/user.py
index bb4d6aed2..cfb34e44e 100644
--- a/src/yunohost/user.py
+++ b/src/yunohost/user.py
@@ -205,32 +205,34 @@ def user_create(operation_logger, username, firstname, lastname, mail, password,
             except IOError as e:
                 raise YunohostError('ssowat_persistent_conf_write_error', error=e.strerror)
 
-    if ldap.add('uid=%s,ou=users' % username, attr_dict):
-        # Invalidate passwd to take user creation into account
-        subprocess.call(['nscd', '-i', 'passwd'])
+    try:
+        ldap.add('uid=%s,ou=users' % username, attr_dict)
+    except Exception as e:
+        raise YunohostError('user_creation_failed', user=username, error=e)
 
-        try:
-            # Attempt to create user home folder
-            subprocess.check_call(
-                ['su', '-', username, '-c', "''"])
-        except subprocess.CalledProcessError:
-            if not os.path.isdir('/home/{0}'.format(username)):
-                logger.warning(m18n.n('user_home_creation_failed'),
-                               exc_info=1)
+    # Invalidate passwd to take user creation into account
+    subprocess.call(['nscd', '-i', 'passwd'])
 
-        # Create group for user and add to group 'all_users'
-        user_group_create(groupname=username, gid=uid, primary_group=True, sync_perm=False)
-        user_group_update(groupname='all_users', add=username, force=True, sync_perm=True)
+    try:
+        # Attempt to create user home folder
+        subprocess.check_call(
+            ['su', '-', username, '-c', "''"])
+    except subprocess.CalledProcessError:
+        if not os.path.isdir('/home/{0}'.format(username)):
+            logger.warning(m18n.n('user_home_creation_failed'),
+                           exc_info=1)
 
-        # TODO: Send a welcome mail to user
-        logger.success(m18n.n('user_created'))
+    # Create group for user and add to group 'all_users'
+    user_group_create(groupname=username, gid=uid, primary_group=True, sync_perm=False)
+    user_group_update(groupname='all_users', add=username, force=True, sync_perm=True)
 
-        hook_callback('post_user_create',
-                      args=[username, mail, password, firstname, lastname])
+    # TODO: Send a welcome mail to user
+    logger.success(m18n.n('user_created'))
 
-        return {'fullname': fullname, 'username': username, 'mail': mail}
+    hook_callback('post_user_create',
+                  args=[username, mail, password, firstname, lastname])
 
-    raise YunohostError('user_creation_failed')
+    return {'fullname': fullname, 'username': username, 'mail': mail}
 
 
 @is_unit_operation([('username', 'user')])
@@ -258,15 +260,17 @@ def user_delete(operation_logger, username, purge=False):
     user_group_delete(username, force=True, sync_perm=True)
 
     ldap = _get_ldap_interface()
-    if ldap.remove('uid=%s,ou=users' % username):
-        # Invalidate passwd to take user deletion into account
-        subprocess.call(['nscd', '-i', 'passwd'])
+    try:
+        ldap.remove('uid=%s,ou=users' % username)
+    except Exception as e:
+        raise YunohostError('user_deletion_failed', user=username, error=e)
 
-        if purge:
-            subprocess.call(['rm', '-rf', '/home/{0}'.format(username)])
-            subprocess.call(['rm', '-rf', '/var/mail/{0}'.format(username)])
-    else:
-        raise YunohostError('user_deletion_failed')
+    # Invalidate passwd to take user deletion into account
+    subprocess.call(['nscd', '-i', 'passwd'])
+
+    if purge:
+        subprocess.call(['rm', '-rf', '/home/{0}'.format(username)])
+        subprocess.call(['rm', '-rf', '/var/mail/{0}'.format(username)])
 
     hook_callback('post_user_delete', args=[username, purge])
 
@@ -387,12 +391,14 @@ def user_update(operation_logger, username, firstname=None, lastname=None, mail=
 
     operation_logger.start()
 
-    if ldap.update('uid=%s,ou=users' % username, new_attr_dict):
-        logger.success(m18n.n('user_updated'))
-        app_ssowatconf()
-        return user_info(username)
-    else:
-        raise YunohostError('user_update_failed')
+    try:
+        ldap.update('uid=%s,ou=users' % username, new_attr_dict)
+    except Exception as e:
+        raise YunohostError('user_update_failed', user=username, error=e)
+
+    logger.success(m18n.n('user_updated'))
+    app_ssowatconf()
+    return user_info(username)
 
 
 def user_info(username):
@@ -476,10 +482,7 @@ def user_info(username):
             'use': storage_use
         }
 
-    if result:
-        return result_dict
-    else:
-        raise YunohostError('user_info_failed')
+    return result_dict
 
 
 #
@@ -569,13 +572,16 @@ def user_group_create(operation_logger, groupname, gid=None, primary_group=False
         attr_dict["member"] = ["uid=" + groupname + ",ou=users,dc=yunohost,dc=org"]
 
     operation_logger.start()
-    if ldap.add('cn=%s,ou=groups' % groupname, attr_dict):
-        logger.success(m18n.n('group_created', group=groupname))
-        if sync_perm:
-            permission_sync_to_user()
-        return {'name': groupname}
+    try:
+        ldap.add('cn=%s,ou=groups' % groupname, attr_dict)
+    except Exception as e:
+        raise YunohostError('group_creation_failed', group=groupname, error=e)
 
-    raise YunohostError('group_creation_failed', group=groupname)
+    if sync_perm:
+        permission_sync_to_user()
+
+    logger.success(m18n.n('group_created', group=groupname))
+    return {'name': groupname}
 
 
 @is_unit_operation([('groupname', 'group')])
@@ -601,13 +607,16 @@ def user_group_delete(operation_logger, groupname, force=False, sync_perm=True):
 
     operation_logger.start()
     ldap = _get_ldap_interface()
-    if not ldap.remove('cn=%s,ou=groups' % groupname):
-        raise YunohostError('group_deletion_failed', group=groupname)
+    try:
+        ldap.remove('cn=%s,ou=groups' % groupname)
+    except Exception as e:
+        raise YunohostError('group_deletion_failed', group=groupname, error=e)
 
-    logger.success(m18n.n('group_deleted', group=groupname))
     if sync_perm:
         permission_sync_to_user()
 
+    logger.success(m18n.n('group_deleted', group=groupname))
+
 
 @is_unit_operation([('groupname', 'group')])
 def user_group_update(operation_logger, groupname, add=None, remove=None, force=False, sync_perm=True):
@@ -668,8 +677,10 @@ def user_group_update(operation_logger, groupname, add=None, remove=None, force=
     if set(new_group) != set(current_group):
         operation_logger.start()
         ldap = _get_ldap_interface()
-        if not ldap.update('cn=%s,ou=groups' % groupname, {"member": set(new_group_dns), "memberUid": set(new_group)}):
-            raise YunohostError('group_update_failed', group=groupname)
+        try:
+            ldap.update('cn=%s,ou=groups' % groupname, {"member": set(new_group_dns), "memberUid": set(new_group)})
+        except Exception as e:
+            raise YunohostError('group_update_failed', group=groupname, error=e)
 
     logger.success(m18n.n('group_updated', group=groupname))
     if sync_perm:

From ec5069b71cd4765c028c593c3cd02da236e0ae2c Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Fri, 13 Sep 2019 15:35:27 +0200
Subject: [PATCH 31/45] Propagate changes on backup tests + fixes bugs found in
 the process

---
 data/hooks/restore/21-conf_ynh_certs          |  1 -
 src/yunohost/backup.py                        | 15 ++++++------
 .../0011_setup_group_permission.py            |  5 ++++
 src/yunohost/tests/test_backuprestore.py      | 23 +++++++++----------
 4 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/data/hooks/restore/21-conf_ynh_certs b/data/hooks/restore/21-conf_ynh_certs
index d1eb532ed..34e651319 100644
--- a/data/hooks/restore/21-conf_ynh_certs
+++ b/data/hooks/restore/21-conf_ynh_certs
@@ -3,6 +3,5 @@ backup_dir="$1/conf/ynh/certs"
 sudo mkdir -p /etc/yunohost/certs/
 
 sudo cp -a $backup_dir/. /etc/yunohost/certs/
-sudo yunohost app ssowatconf
 sudo service nginx reload
 sudo service metronome reload
diff --git a/src/yunohost/backup.py b/src/yunohost/backup.py
index f1ac7ee9c..de2b3f76d 100644
--- a/src/yunohost/backup.py
+++ b/src/yunohost/backup.py
@@ -1134,6 +1134,8 @@ class RestoreManager():
 
             self._restore_system()
             self._restore_apps()
+        except Exception as e:
+            logger.error("The following critical error happened during restoration: %s" % e)
         finally:
             self.clean()
 
@@ -1186,11 +1188,12 @@ class RestoreManager():
         if system_targets == []:
             return
 
-        from yunohost.permission import permission_create, user_permission_update, user_permission_list
+        from yunohost.user import user_group_list
+        from yunohost.permission import permission_create, permission_delete, user_permission_update, user_permission_list
 
         # Backup old permission for apps
         # We need to do that because in case of an app is installed we can't remove the permission for this app
-        old_apps_permission = user_permission_list(ignore_system_perms=True)["permissions"]
+        old_apps_permission = user_permission_list(ignore_system_perms=True, full=True)["permissions"]
 
         # Start register change on system
         operation_logger = OperationLogger('backup_restore_system')
@@ -1232,7 +1235,7 @@ class RestoreManager():
         # do the migration 0011 : setup group and permission
         #
         # Legacy code
-        if not user_group_list["groups"]:
+        if not "all_users" in user_group_list()["groups"].keys():
             from yunohost.tools import _get_migration_by_name
             setup_group_permission = _get_migration_by_name("setup_group_permission")
             # Update LDAP schema restart slapd
@@ -1251,14 +1254,12 @@ class RestoreManager():
                 permission_create(permission_name, urls=permission_infos["urls"], sync_perm=False)
                 user_permission_update(permission_name, remove="all_users", add=permission_infos["allowed"])
 
-
     def _restore_apps(self):
         """Restore all apps targeted"""
 
         apps_targets = self.targets.list("apps", exclude=["Skipped"])
 
         for app in apps_targets:
-            print(app)
             self._restore_app(app)
 
     def _restore_app(self, app_instance_name):
@@ -1359,11 +1360,11 @@ class RestoreManager():
                 permissions = read_yaml('%s/permissions.yml' % app_settings_new_path)
                 existing_groups = user_group_list()['groups']
 
-                for permission_name, permission_infos in permissions:
+                for permission_name, permission_infos in permissions.items():
 
                     permission_create(permission_name, urls=permission_infos.get("urls", []))
 
-                    if "allowed" not in permissions_infos:
+                    if "allowed" not in permission_infos:
                         logger.warning("'allowed' key corresponding to allowed groups for permission %s not found when restoring app %s ... You might need to reconfigure permissions yourself!" % (permission_name, app_instance_name))
                     else:
                         groups = [g for g in permission_infos["allowed"] if g in existing_groups]
diff --git a/src/yunohost/data_migrations/0011_setup_group_permission.py b/src/yunohost/data_migrations/0011_setup_group_permission.py
index 720e4ac36..109757bcc 100644
--- a/src/yunohost/data_migrations/0011_setup_group_permission.py
+++ b/src/yunohost/data_migrations/0011_setup_group_permission.py
@@ -89,6 +89,11 @@ class MyMigration(Migration):
             app_setting(app, 'allowed_users', delete=True)
 
     def run(self):
+
+        # FIXME : what do we really want to do here ...
+        # Imho we should just force-regen the conf in all case, and maybe
+        # just display a warning if we detect that the conf was manually modified
+
         # Check if the migration can be processed
         ldap_regen_conf_status = regen_conf(names=['slapd'], dry_run=True)
         # By this we check if the have been customized
diff --git a/src/yunohost/tests/test_backuprestore.py b/src/yunohost/tests/test_backuprestore.py
index 7d384a46a..d2fd03799 100644
--- a/src/yunohost/tests/test_backuprestore.py
+++ b/src/yunohost/tests/test_backuprestore.py
@@ -38,10 +38,10 @@ def setup_function(function):
         add_archive_wordpress_from_2p4()
         assert len(backup_list()["archives"]) == 1
 
-    if "with_backup_legacy_app_installed" in markers:
-        assert not app_is_installed("backup_legacy_app")
-        install_app("backup_legacy_app_ynh", "/yolo")
-        assert app_is_installed("backup_legacy_app")
+    if "with_legacy_app_installed" in markers:
+        assert not app_is_installed("legacy_app")
+        install_app("legacy_app_ynh", "/yolo")
+        assert app_is_installed("legacy_app")
 
     if "with_backup_recommended_app_installed" in markers:
         assert not app_is_installed("backup_recommended_app")
@@ -105,7 +105,7 @@ def backup_test_dependencies_are_met():
 
     # Dummy test apps (or backup archives)
     assert os.path.exists("./tests/apps/backup_wordpress_from_2p4")
-    assert os.path.exists("./tests/apps/backup_legacy_app_ynh")
+    assert os.path.exists("./tests/apps/legacy_app_ynh")
     assert os.path.exists("./tests/apps/backup_recommended_app_ynh")
 
     return True
@@ -155,8 +155,8 @@ def delete_all_backups():
 
 def uninstall_test_apps_if_needed():
 
-    if _is_installed("backup_legacy_app"):
-        app_remove("backup_legacy_app")
+    if _is_installed("legacy_app"):
+        app_remove("legacy_app")
 
     if _is_installed("backup_recommended_app"):
         app_remove("backup_recommended_app")
@@ -497,10 +497,10 @@ def test_restore_app_already_installed(mocker):
     assert _is_installed("wordpress")
 
 
-@pytest.mark.with_backup_legacy_app_installed
+@pytest.mark.with_legacy_app_installed
 def test_backup_and_restore_legacy_app():
 
-    _test_backup_and_restore_app("backup_legacy_app")
+    _test_backup_and_restore_app("legacy_app")
 
 
 @pytest.mark.with_backup_recommended_app_installed
@@ -531,7 +531,7 @@ def _test_backup_and_restore_app(app):
     # Uninstall the app
     app_remove(app)
     assert not app_is_installed(app)
-    assert app not in user_permission_list()['permissions']
+    assert app+".main" not in user_permission_list()['permissions']
 
     # Restore the app
     backup_restore(system=None, name=archives[0],
@@ -541,8 +541,7 @@ def _test_backup_and_restore_app(app):
 
     # Check permission
     per_list = user_permission_list()['permissions']
-    assert app in per_list
-    assert "main" in per_list[app]
+    assert app+".main" in per_list
 
 #
 # Some edge cases                                                            #

From ccc7583ec4e909e4c2d06511f32310e6d3abba6c Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Fri, 13 Sep 2019 16:02:02 +0200
Subject: [PATCH 32/45] Add backup/restore test for permission app, and fix a
 small related bug

---
 src/yunohost/backup.py                   |  6 ++-
 src/yunohost/tests/test_backuprestore.py | 55 ++++++++++++++++++++----
 2 files changed, 50 insertions(+), 11 deletions(-)

diff --git a/src/yunohost/backup.py b/src/yunohost/backup.py
index de2b3f76d..420c2d4f8 100644
--- a/src/yunohost/backup.py
+++ b/src/yunohost/backup.py
@@ -1367,8 +1367,10 @@ class RestoreManager():
                     if "allowed" not in permission_infos:
                         logger.warning("'allowed' key corresponding to allowed groups for permission %s not found when restoring app %s ... You might need to reconfigure permissions yourself!" % (permission_name, app_instance_name))
                     else:
-                        groups = [g for g in permission_infos["allowed"] if g in existing_groups]
-                        user_permission_update(permission_name, remove="all_users", add=groups)
+                        should_be_allowed = [g for g in permission_infos["allowed"] if g in existing_groups]
+                        current_allowed = user_permission_list()["permissions"][permission_name]["allowed"]
+                        if should_be_allowed != current_allowed:
+                            user_permission_update(permission_name, remove=current_allowed, add=should_be_allowed)
 
                 os.remove('%s/permissions.yml' % app_settings_new_path)
             else:
diff --git a/src/yunohost/tests/test_backuprestore.py b/src/yunohost/tests/test_backuprestore.py
index d2fd03799..bdaf25299 100644
--- a/src/yunohost/tests/test_backuprestore.py
+++ b/src/yunohost/tests/test_backuprestore.py
@@ -10,7 +10,7 @@ from yunohost.app import _is_installed
 from yunohost.backup import backup_create, backup_restore, backup_list, backup_info, backup_delete, _recursive_umount
 from yunohost.domain import _get_maindomain
 from yunohost.utils.error import YunohostError
-from yunohost.user import user_permission_list
+from yunohost.user import user_permission_list, user_create, user_list, user_delete
 from yunohost.tests.test_permission import check_LDAP_db_integrity, check_permission_for_apps
 
 # Get main domain
@@ -59,6 +59,13 @@ def setup_function(function):
         add_archive_system_from_2p4()
         assert len(backup_list()["archives"]) == 1
 
+    if "with_permission_app_installed" in markers:
+        assert not app_is_installed("permissions_app")
+        user_create("alice", "Alice", "White", "alice@" + maindomain, "test123Ynh")
+        install_app("permissions_app_ynh", "/urlpermissionapp"
+                    "&admin=alice")
+        assert app_is_installed("permissions_app")
+
 
 def teardown_function(function):
 
@@ -73,6 +80,9 @@ def teardown_function(function):
     if "clean_opt_dir" in markers:
         shutil.rmtree("/opt/test_backup_output_directory")
 
+    if "alice" in user_list()["users"]:
+        user_delete("alice")
+
 
 @pytest.fixture(autouse=True)
 def check_LDAP_db_integrity_call():
@@ -92,6 +102,9 @@ def check_permission_for_apps_call():
 
 def app_is_installed(app):
 
+    if app == "permissions_app":
+        return _is_installed(app)
+
     # These are files we know should be installed by the app
     app_files = []
     app_files.append("/etc/nginx/conf.d/%s.d/%s.conf" % (maindomain, app))
@@ -155,14 +168,9 @@ def delete_all_backups():
 
 def uninstall_test_apps_if_needed():
 
-    if _is_installed("legacy_app"):
-        app_remove("legacy_app")
-
-    if _is_installed("backup_recommended_app"):
-        app_remove("backup_recommended_app")
-
-    if _is_installed("wordpress"):
-        app_remove("wordpress")
+    for app in ["legacy_app", "backup_recommended_app", "wordpress", "permissions_app"]:
+        if _is_installed(app):
+            app_remove(app)
 
 
 def install_app(app, path, additionnal_args=""):
@@ -514,6 +522,35 @@ def test_backup_and_restore_with_ynh_restore():
 
     _test_backup_and_restore_app("backup_recommended_app")
 
+@pytest.mark.with_permission_app_installed
+def test_backup_and_restore_permission_app():
+
+    res = user_permission_list(full=True)['permissions']
+    assert "permissions_app.main" in res
+    assert "permissions_app.admin" in res
+    assert "permissions_app.dev" in res
+    assert res['permissions_app.main']['urls'] == [maindomain + "/urlpermissionapp"]
+    assert res['permissions_app.admin']['urls'] == [maindomain + "/urlpermissionapp/admin"]
+    assert res['permissions_app.dev']['urls'] == [maindomain + "/urlpermissionapp/dev"]
+
+    assert res['permissions_app.main']['allowed'] == ["all_users"]
+    assert res['permissions_app.admin']['allowed'] == ["alice"]
+    assert res['permissions_app.dev']['allowed'] == []
+
+    _test_backup_and_restore_app("permissions_app")
+
+    res = user_permission_list(full=True)['permissions']
+    assert "permissions_app.main" in res
+    assert "permissions_app.admin" in res
+    assert "permissions_app.dev" in res
+    assert res['permissions_app.main']['urls'] == [maindomain + "/urlpermissionapp"]
+    assert res['permissions_app.admin']['urls'] == [maindomain + "/urlpermissionapp/admin"]
+    assert res['permissions_app.dev']['urls'] == [maindomain + "/urlpermissionapp/dev"]
+
+    assert res['permissions_app.main']['allowed'] == ["all_users"]
+    assert res['permissions_app.admin']['allowed'] == ["alice"]
+    assert res['permissions_app.dev']['allowed'] == []
+
 
 def _test_backup_and_restore_app(app):
 

From 302e755f48f853cf1362eb577885c990cc434ca7 Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Fri, 13 Sep 2019 16:50:46 +0200
Subject: [PATCH 33/45] Assume we target the .main permission if it's not given
 explicitly

---
 data/actionsmap/yunohost.yml |  4 ++--
 src/yunohost/permission.py   | 32 ++++++++++++++++++++++++--------
 2 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/data/actionsmap/yunohost.yml b/data/actionsmap/yunohost.yml
index 49dde373b..05f0de048 100644
--- a/data/actionsmap/yunohost.yml
+++ b/data/actionsmap/yunohost.yml
@@ -298,7 +298,7 @@ user:
                     api: POST /users/permissions/<permission>
                     arguments:
                         permission:
-                            help: Permission to manage (e.g. mail.main or wordpress.editors)
+                            help: Permission to manage (e.g. mail or nextcloud or wordpress.editors)
                         -a:
                             full: --add
                             help: Group or user names to add to this permission
@@ -320,7 +320,7 @@ user:
                     api: DELETE /users/permissions/<app>
                     arguments:
                         permission:
-                            help: Permission to be resetted (e.g. mail.main or wordpress.editors)
+                            help: Permission to manage (e.g. mail or nextcloud or wordpress.editors)
 
         ssh:
             subcategory_help: Manage ssh access
diff --git a/src/yunohost/permission.py b/src/yunohost/permission.py
index 984a5d902..1472f4b88 100644
--- a/src/yunohost/permission.py
+++ b/src/yunohost/permission.py
@@ -87,15 +87,19 @@ def user_permission_update(operation_logger, permission, add=None, remove=None,
     Allow or Disallow a user or group to a permission for a specific application
 
     Keyword argument:
-        permission     -- Name of the permission (e.g. mail.mail or wordpress.editors)
+        permission     -- Name of the permission (e.g. mail or or wordpress or wordpress.editors)
         add            -- List of groups or usernames to add to this permission
         remove         -- List of groups or usernames to remove from to this permission
     """
     from yunohost.hook import hook_callback
     from yunohost.user import user_group_list
-    from yunohost.utils.ldap import _get_ldap_interface, _ldap_path_extract
+    from yunohost.utils.ldap import _get_ldap_interface
     ldap = _get_ldap_interface()
 
+    # By default, manipulate main permission
+    if "." not in permission:
+        permission = permission + ".main"
+
     # Fetch currently allowed groups for this permission
 
     existing_permission = user_permission_list(full=True)["permissions"].get(permission, None)
@@ -146,7 +150,7 @@ def user_permission_update(operation_logger, permission, add=None, remove=None,
     # Don't update LDAP if we update exactly the same values
     if set(new_allowed_groups) == set(current_allowed_groups):
         # FIXME : i18n
-        logger.warning("No change was applied because not relevant modification were found")
+        logger.warning("The permission was not updated all addition/removal requests already match the current state.")
         return
 
     # Commit the new allowed group list
@@ -192,12 +196,16 @@ def user_permission_reset(operation_logger, permission, sync_perm=True):
     Reset a given permission to just 'all_users'
 
     Keyword argument:
-        permission -- The name of the permission to be reseted
+        permission -- Name of the permission (e.g. mail or nextcloud or wordpress.editors)
     """
     from yunohost.hook import hook_callback
     from yunohost.utils.ldap import _get_ldap_interface
     ldap = _get_ldap_interface()
 
+    # By default, manipulate main permission
+    if "." not in permission:
+        permission = permission + ".main"
+
     # Fetch existing permission
 
     existing_permission = user_permission_list(full=True)["permissions"].get(permission, None)
@@ -254,13 +262,17 @@ def permission_create(operation_logger, permission, urls=None, sync_perm=True):
     Create a new permission for a specific application
 
     Keyword argument:
-        permission -- Name of the permission (e.g. nextcloud.main or wordpress.editors)
+        permission -- Name of the permission (e.g. mail or nextcloud or wordpress.editors)
         urls       -- list of urls to specify for the permission
     """
 
     from yunohost.utils.ldap import _get_ldap_interface
     ldap = _get_ldap_interface()
 
+    # By default, manipulate main permission
+    if "." not in permission:
+        permission = permission + ".main"
+
     # Validate uniqueness of permission in LDAP
     if ldap.get_conflict({'cn': permission},
                          base_dn='ou=permission,dc=yunohost,dc=org'):
@@ -308,7 +320,7 @@ def permission_urls(operation_logger, permission, add=None, remove=None, sync_pe
     Update urls related to a permission for a specific application
 
     Keyword argument:
-        permission -- Name of the permission (e.g. nextcloud.main or wordpress.editors)
+        permission -- Name of the permission (e.g. mail or nextcloud or wordpress.editors)
         add        -- List of urls to add
         remove     -- List of urls to remove
 
@@ -362,10 +374,14 @@ def permission_delete(operation_logger, permission, force=False, sync_perm=True)
     Delete a permission
 
     Keyword argument:
-        permission -- Name of the permission (e.g. nextcloud.main or wordpress.editors)
+        permission -- Name of the permission (e.g. mail or nextcloud or wordpress.editors)
     """
 
-    if permission.endswith("main") and not force:
+    # By default, manipulate main permission
+    if "." not in permission:
+        permission = permission + ".main"
+
+    if permission.endswith(".main") and not force:
         raise YunohostError('permission_cannot_remove_main')
 
     from yunohost.utils.ldap import _get_ldap_interface

From f950378c63aec6a2536524e3e02c3cfda86a09b9 Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Fri, 13 Sep 2019 17:39:21 +0200
Subject: [PATCH 34/45] Do not display primary groups by default when running
 yunohost user group list

---
 data/actionsmap/yunohost.yml |  5 +++++
 src/yunohost/user.py         | 11 ++++++++++-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/data/actionsmap/yunohost.yml b/data/actionsmap/yunohost.yml
index 05f0de048..1f6966f65 100644
--- a/data/actionsmap/yunohost.yml
+++ b/data/actionsmap/yunohost.yml
@@ -216,6 +216,11 @@ user:
                             full: --full
                             help: Display all informations known about each groups
                             action: store_true
+                        -p:
+                            full: --include-primary-groups
+                            help: Also display primary groups (each user has an eponym group that only contains itself)
+                            action: store_true
+                            default: false
 
                 ### user_group_create()
                 create:
diff --git a/src/yunohost/user.py b/src/yunohost/user.py
index cfb34e44e..4fe8db420 100644
--- a/src/yunohost/user.py
+++ b/src/yunohost/user.py
@@ -488,13 +488,17 @@ def user_info(username):
 #
 # Group subcategory
 #
-def user_group_list(short=False, full=False):
+def user_group_list(short=False, full=False, include_primary_groups=True):
     """
     List users
 
     Keyword argument:
         short -- Only list the name of the groups without any additional info
         full -- List all the info available for each groups
+        include_primary_groups -- Include groups corresponding to users (which should always only contains this user)
+                                  This option is set to false by default in the action map because we don't want to have
+                                  these displayed when the user runs `yunohost user group list`, but internally we do want
+                                  to list them when called from other functions
     """
 
     # Fetch relevant informations
@@ -507,10 +511,15 @@ def user_group_list(short=False, full=False):
 
     # Parse / organize information to be outputed
 
+    users = user_list()["users"]
     groups = {}
     for infos in groups_infos:
 
         name = infos["cn"][0]
+
+        if not include_primary_groups and name in users:
+            continue
+
         groups[name] = {}
 
         groups[name]["members"] = [_ldap_path_extract(p, "uid") for p in infos.get("member", [])]

From ea8c0cae9431bd294f838448eb297d84ee6fd5a4 Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Fri, 13 Sep 2019 18:34:26 +0200
Subject: [PATCH 35/45] Deprecate legacy app access system

---
 data/actionsmap/yunohost.yml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/data/actionsmap/yunohost.yml b/data/actionsmap/yunohost.yml
index 1f6966f65..e51f23a14 100644
--- a/data/actionsmap/yunohost.yml
+++ b/data/actionsmap/yunohost.yml
@@ -792,6 +792,7 @@ app:
         addaccess:
             action_help: Grant access right to users (everyone by default)
             api: PUT /access
+            deprecated: true
             arguments:
                 apps:
                     nargs: "+"
@@ -803,6 +804,7 @@ app:
         removeaccess:
             action_help: Revoke access right to users (everyone by default)
             api: DELETE /access
+            deprecated: true
             arguments:
                 apps:
                     nargs: "+"
@@ -814,6 +816,7 @@ app:
         clearaccess:
             action_help: Reset access rights for the app
             api: POST /access
+            deprecated: true
             arguments:
                 apps:
                     nargs: "+"

From b995b3254d95008f51591a76d8bbbcfab7bc260c Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Fri, 13 Sep 2019 18:41:05 +0200
Subject: [PATCH 36/45] Remove some unecessary messages when handling primary
 groups and all_users

---
 src/yunohost/user.py | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/src/yunohost/user.py b/src/yunohost/user.py
index 4fe8db420..22bd6d3cf 100644
--- a/src/yunohost/user.py
+++ b/src/yunohost/user.py
@@ -589,7 +589,11 @@ def user_group_create(operation_logger, groupname, gid=None, primary_group=False
     if sync_perm:
         permission_sync_to_user()
 
-    logger.success(m18n.n('group_created', group=groupname))
+    if not primary_group:
+        logger.success(m18n.n('group_created', group=groupname))
+    else:
+        logger.debug(m18n.n('group_created', group=groupname))
+
     return {'name': groupname}
 
 
@@ -624,7 +628,10 @@ def user_group_delete(operation_logger, groupname, force=False, sync_perm=True):
     if sync_perm:
         permission_sync_to_user()
 
-    logger.success(m18n.n('group_deleted', group=groupname))
+    if groupname not in existing_users:
+        logger.success(m18n.n('group_deleted', group=groupname))
+    else:
+        logger.debug(m18n.n('group_deleted', group=groupname))
 
 
 @is_unit_operation([('groupname', 'group')])
@@ -691,7 +698,11 @@ def user_group_update(operation_logger, groupname, add=None, remove=None, force=
         except Exception as e:
             raise YunohostError('group_update_failed', group=groupname, error=e)
 
-    logger.success(m18n.n('group_updated', group=groupname))
+    if groupname != "all_users":
+        logger.success(m18n.n('group_updated', group=groupname))
+    else:
+        logger.debug(m18n.n('group_updated', group=groupname))
+
     if sync_perm:
         permission_sync_to_user()
     return user_group_info(groupname)

From 732f8987738bfb585de14a0922d374e7c2c616e4 Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Fri, 13 Sep 2019 19:42:15 +0200
Subject: [PATCH 37/45] Small issue when deleting the user

---
 src/yunohost/user.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/yunohost/user.py b/src/yunohost/user.py
index 22bd6d3cf..fbd15018c 100644
--- a/src/yunohost/user.py
+++ b/src/yunohost/user.py
@@ -253,6 +253,8 @@ def user_delete(operation_logger, username, purge=False):
 
     user_group_update("all_users", remove=username, force=True, sync_perm=False)
     for group, infos in user_group_list()["groups"].items():
+        if group == "all_users":
+            continue
         # If the user is in this group (and it's not the primary group),
         # remove the member from the group
         if username != group and username in infos["members"]:

From 63fa54171de033a1fe82612a97511e0579d3eff1 Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Fri, 13 Sep 2019 20:13:44 +0200
Subject: [PATCH 38/45] Ugh we really need to make this raise an exception ...

---
 src/yunohost/backup.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/yunohost/backup.py b/src/yunohost/backup.py
index 420c2d4f8..305152865 100644
--- a/src/yunohost/backup.py
+++ b/src/yunohost/backup.py
@@ -1135,7 +1135,7 @@ class RestoreManager():
             self._restore_system()
             self._restore_apps()
         except Exception as e:
-            logger.error("The following critical error happened during restoration: %s" % e)
+            raise YunohostError("The following critical error happened during restoration: %s" % e)
         finally:
             self.clean()
 
@@ -1245,7 +1245,7 @@ class RestoreManager():
 
         # Remove all permission for all app which is still in the LDAP
         for permission_name in user_permission_list(ignore_system_perms=True)["permissions"].keys():
-            permission_delete(permission_name)
+            permission_delete(permission_name, force=True)
 
         # Restore permission for the app which is installed
         for permission_name, permission_infos in old_apps_permission.items():

From 3df6ce17b6b13f1fe784a3bb9e5b782fc541eb2a Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Fri, 13 Sep 2019 20:34:30 +0200
Subject: [PATCH 39/45] 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...
     #

From 094a2afe1a7a5eb21c9fdcb51ab79de05c9589a7 Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Fri, 13 Sep 2019 22:45:31 +0200
Subject: [PATCH 40/45] Simplify permission handling in app_map + add tests for
 it

---
 src/yunohost/app.py                   | 10 +++-------
 src/yunohost/tests/test_permission.py | 15 +++++++++++----
 2 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/src/yunohost/app.py b/src/yunohost/app.py
index b3c36d059..ab290cb4d 100644
--- a/src/yunohost/app.py
+++ b/src/yunohost/app.py
@@ -406,10 +406,10 @@ def app_map(app=None, raw=False, user=None):
 
     """
     from yunohost.permission import user_permission_list
-    from yunohost.utils.ldap import _get_ldap_interface
 
     apps = []
     result = {}
+    permissions = user_permission_list(full=True)["permissions"]
 
     if app is not None:
         if not _is_installed(app):
@@ -429,12 +429,8 @@ def app_map(app=None, raw=False, user=None):
             continue
         if 'no_sso' in app_settings:  # I don't think we need to check for the value here
             continue
-        if user is not None:
-            ldap = _get_ldap_interface()
-            if not ldap.search(base='ou=permission,dc=yunohost,dc=org',
-                               filter='(&(objectclass=permissionYnh)(cn=%s.main)(inheritPermission=uid=%s,ou=users,dc=yunohost,dc=org))' % (app_id, user),
-                               attrs=['cn']):
-                continue
+        if user and user not in permissions[app_id + ".main"]["corresponding_users"]:
+            continue
 
         domain = app_settings['domain']
         path = app_settings['path']
diff --git a/src/yunohost/tests/test_permission.py b/src/yunohost/tests/test_permission.py
index 8db1ae825..94728505d 100644
--- a/src/yunohost/tests/test_permission.py
+++ b/src/yunohost/tests/test_permission.py
@@ -1,6 +1,6 @@
 import pytest
 
-from yunohost.app import app_install, app_remove, app_change_url, app_list
+from yunohost.app import app_install, app_remove, app_change_url, app_list, app_map
 
 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
@@ -331,7 +331,7 @@ def test_permission_remove_url_not_added():
 # Application interaction
 #
 
-def test_install_app():
+def test_permission_app_install():
     app_install("./tests/apps/permissions_app_ynh",
                 args="domain=%s&path=%s&admin=%s" % (maindomain, "/urlpermissionapp", "alice"), force=True)
 
@@ -352,7 +352,14 @@ def test_install_app():
     assert res['permissions_app.dev']['allowed'] == []
     assert set(res['permissions_app.dev']['corresponding_users']) == set()
 
-def test_remove_app():
+    # Check that we get the right stuff in app_map, which is used to generate the ssowatconf
+    assert maindomain + "/urlpermissionapp" in app_map(user="alice").keys()
+    user_permission_update("permissions_app.main", remove="all_users", add="bob")
+    assert maindomain + "/urlpermissionapp" not in app_map(user="alice").keys()
+    assert maindomain + "/urlpermissionapp" in app_map(user="bob").keys()
+
+
+def test_permission_app_remove():
     app_install("./tests/apps/permissions_app_ynh",
                 args="domain=%s&path=%s&admin=%s" % (maindomain, "/urlpermissionapp", "alice"), force=True)
     app_remove("permissions_app")
@@ -361,7 +368,7 @@ def test_remove_app():
     res = user_permission_list(full=True)['permissions']
     assert not any(p.startswith("permissions_app.") for p in res.keys())
 
-def test_change_url():
+def test_permission_app_change_url():
     app_install("./tests/apps/permissions_app_ynh",
                 args="domain=%s&path=%s&admin=%s" % (maindomain, "/urlpermissionapp", "alice"), force=True)
 

From 9c383ef06a106505d3f726276d6d2cfba4e4cc41 Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Sat, 14 Sep 2019 18:21:42 +0200
Subject: [PATCH 41/45] Make migration more robust to re-runs

---
 locales/en.json                               |  2 +-
 .../0011_setup_group_permission.py            | 31 ++++++++++++++++---
 2 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/locales/en.json b/locales/en.json
index c48532ed7..ae349edf3 100644
--- a/locales/en.json
+++ b/locales/en.json
@@ -354,7 +354,6 @@
     "migration_0011_can_not_backup_before_migration": "The backup of the system before the migration failed. Migration failed. Error: {error:s}",
     "migration_0011_create_group": "Creating a group for each user...",
     "migration_0011_done": "Migration successful. You are now able to manage groups of users.",
-    "migration_0011_error_when_removing_sftpuser_group": "Error when trying remove sftpusers group",
     "migration_0011_LDAP_config_dirty": "It look like that you customized your LDAP configuration. For this migration the LDAP configuration need to be updated.\nYou need to save your actual configuration, reintialize the original configuration by the command 'yunohost tools regen-conf -f' and after retry the migration",
     "migration_0011_LDAP_update_failed": "LDAP update failed. Error: {error:s}",
     "migration_0011_migrate_permission": "Migrating permissions from apps settings to LDAP...",
@@ -362,6 +361,7 @@
     "migration_0011_rollback_success": "Rollback succeeded.",
     "migration_0011_update_LDAP_database": "Updating LDAP database...",
     "migration_0011_update_LDAP_schema": "Updating LDAP schema...",
+    "migration_0011_failed_to_remove_stale_object": "Failed to remove stale object {dn}: {error}",
     "migrations_already_ran": "Those migrations have already been ran: {ids}",
     "migrations_cant_reach_migration_file": "Can't access migrations files at path %s",
     "migrations_dependencies_not_satisfied": "Can't run migration {id} because first you need to run these migrations: {dependencies_id}",
diff --git a/src/yunohost/data_migrations/0011_setup_group_permission.py b/src/yunohost/data_migrations/0011_setup_group_permission.py
index 109757bcc..8949239e0 100644
--- a/src/yunohost/data_migrations/0011_setup_group_permission.py
+++ b/src/yunohost/data_migrations/0011_setup_group_permission.py
@@ -28,6 +28,28 @@ class MyMigration(Migration):
 
     required = True
 
+    def remove_if_exists(self, target):
+
+        from yunohost.utils.ldap import _get_ldap_interface
+        ldap = _get_ldap_interface()
+
+        try:
+            objects = ldap.search(target + ",dc=yunohost,dc=org")
+        # ldap search will raise an exception if no corresponding object is found >.> ...
+        except Exception as e:
+            logger.debug("%s does not exist, no need to delete it" % target)
+            return
+
+        objects.reverse()
+        for o in objects:
+            for dn in o["dn"]:
+                dn = dn.replace(",dc=yunohost,dc=org", "")
+                logger.debug("Deleting old object %s ..." % dn)
+                try:
+                    ldap.remove(dn)
+                except Exception as e:
+                    raise YunohostError("migration_0011_failed_to_remove_stale_object", dn=dn, error=e)
+
     def migrate_LDAP_db(self):
 
         logger.info(m18n.n("migration_0011_update_LDAP_database"))
@@ -35,14 +57,13 @@ class MyMigration(Migration):
         from yunohost.utils.ldap import _get_ldap_interface
         ldap = _get_ldap_interface()
 
-        try:
-            ldap.remove('cn=sftpusers,ou=groups')
-        except:
-            logger.warn(m18n.n("migration_0011_error_when_removing_sftpuser_group"))
-
         ldap_map = read_yaml('/usr/share/yunohost/yunohost-config/moulinette/ldap_scheme.yml')
 
         try:
+            self.remove_if_exists("cn=sftpusers,ou=groups")
+            self.remove_if_exists("ou=permission")
+            self.remove_if_exists('cn=all_users,ou=groups')
+
             attr_dict = ldap_map['parents']['ou=permission']
             ldap.add('ou=permission', attr_dict)
 

From 379c28de909774bac44e10af34ecd6d91e281ab1 Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Fri, 20 Sep 2019 15:00:31 +0200
Subject: [PATCH 42/45] Update src/yunohost/backup.py
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Co-Authored-By: Allan Nordhøy <epost@anotheragency.no>
---
 src/yunohost/backup.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/yunohost/backup.py b/src/yunohost/backup.py
index 305152865..c28160342 100644
--- a/src/yunohost/backup.py
+++ b/src/yunohost/backup.py
@@ -1365,7 +1365,7 @@ class RestoreManager():
                     permission_create(permission_name, urls=permission_infos.get("urls", []))
 
                     if "allowed" not in permission_infos:
-                        logger.warning("'allowed' key corresponding to allowed groups for permission %s not found when restoring app %s ... You might need to reconfigure permissions yourself!" % (permission_name, app_instance_name))
+                        logger.warning("'allowed' key corresponding to allowed groups for permission %s not found when restoring app %s … You might have to reconfigure permissions yourself." % (permission_name, app_instance_name))
                     else:
                         should_be_allowed = [g for g in permission_infos["allowed"] if g in existing_groups]
                         current_allowed = user_permission_list()["permissions"][permission_name]["allowed"]

From 545f697df2378d138ce7af73ef985df170f445e1 Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Fri, 20 Sep 2019 21:56:44 +0200
Subject: [PATCH 43/45] When using the legacy adduser function, remove
 all_users for backward compatibility

---
 src/yunohost/app.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/yunohost/app.py b/src/yunohost/app.py
index ab290cb4d..ae6d27b20 100644
--- a/src/yunohost/app.py
+++ b/src/yunohost/app.py
@@ -1039,7 +1039,7 @@ def app_addaccess(apps, users=[]):
 
     output = {}
     for app in apps:
-        permission = user_permission_update(app+".main", add=users)
+        permission = user_permission_update(app+".main", add=users, remove="all_users")
         output[app] = permission["corresponding_users"]
 
     return {'allowed_users': output}

From e2e960175b5a9839cb1c30587a16f7c1d4a44dd2 Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Sat, 21 Sep 2019 13:16:28 +0200
Subject: [PATCH 44/45] Apply suggestions from code review
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Update descriptions for action map

Co-Authored-By: Allan Nordhøy <epost@anotheragency.no>
---
 data/actionsmap/yunohost.yml | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/data/actionsmap/yunohost.yml b/data/actionsmap/yunohost.yml
index e51f23a14..97489a841 100644
--- a/data/actionsmap/yunohost.yml
+++ b/data/actionsmap/yunohost.yml
@@ -270,11 +270,11 @@ user:
 
                 ### user_group_info()
                 info:
-                    action_help: Get information for a specific group
+                    action_help: Get information about a specific group
                     api: GET /users/groups/<groupname>
                     arguments:
                         groupname:
-                            help: Name of the group to get info about
+                            help: Name of the group to fetch info about
                             extra:
                                 pattern: *pattern_username
 
@@ -289,11 +289,11 @@ user:
                     arguments:
                         -s:
                             full: --short
-                            help: List only the names of permissions
+                            help: Only list permission names
                             action: store_true
                         -f:
                             full: --full
-                            help: Display all informations known about each permissions, including the full list of users corresponding to allowed groups.
+                            help: Display all info known about each permission, including the full user list of each group it is granted to.
                             action: store_true
 
 
@@ -306,14 +306,14 @@ user:
                             help: Permission to manage (e.g. mail or nextcloud or wordpress.editors)
                         -a:
                             full: --add
-                            help: Group or user names to add to this permission
+                            help: Group or usernames to grant this permission to
                             nargs: "*"
                             metavar: GROUP_OR_USER
                             extra:
                                 pattern: *pattern_username
                         -r:
                             full: --remove
-                            help: Group or user names to remove from this permission
+                            help: Group or usernames revoke this permission from
                             nargs: "*"
                             metavar: GROUP_OR_USER
                             extra:

From ea9a93cec097a14f043b9ecc295ea23b7d14629a Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Wed, 25 Sep 2019 00:07:50 +0200
Subject: [PATCH 45/45] Update data/actionsmap/yunohost.yml
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Co-Authored-By: Allan Nordhøy <epost@anotheragency.no>
---
 data/actionsmap/yunohost.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/data/actionsmap/yunohost.yml b/data/actionsmap/yunohost.yml
index 97489a841..22037f05f 100644
--- a/data/actionsmap/yunohost.yml
+++ b/data/actionsmap/yunohost.yml
@@ -299,7 +299,7 @@ user:
 
                 ### user_permission_update()
                 update:
-                    action_help: Grant / remove permissions to groups or users
+                    action_help: Manage group or user permissions
                     api: POST /users/permissions/<permission>
                     arguments:
                         permission: