Moaaar fixes

src/yunohost/permission.py

@@ -98,6 +98,9 @@ def user_permission_list(short=False, full=False, ignore_system_perms=False, ful
         subpermissions = {k: v for k, v in permissions.items() if not k.endswith(".main")}
         for name, infos in subpermissions.items():
             main_perm_name = name.split(".")[0] + ".main"
+            if main_perm_name not in permissions:
+                logger.debug("Uhoh, unknown permission %s ? (Maybe we're in the process or deleting the perm for this app...)" % main_perm_name)
+                continue
             main_perm_label = permissions[main_perm_name]["label"]
             infos["label"] = "%s (%s)" % (main_perm_label, infos["label"])
 
@@ -321,15 +324,16 @@ def permission_create(operation_logger, permission, allowed=None,
         gid = str(random.randint(200, 99999))
         uid_guid_found = gid not in all_gid
 
+    app, subperm = permission.split(".")
+
     attr_dict = {
         'objectClass': ['top', 'permissionYnh', 'posixGroup'],
         'cn': str(permission),
         'gidNumber': gid,
         'authHeader': ['TRUE'],
-        'label': [str(permission.split('.')[0].title() if permission.endswith('.main')
+        'label': [str(label) if label else (subperm if subperm != "main" else app.title())],
-                  else "%s (%s)" % (permission.split('.')[0].title(), permission.split('.')[1]))],
+        'showTile': ['FALSE'],  # Dummy value, it will be fixed when we call '_update_ldap_group_permission'
-        'showTile': ['FALSE'], # Dummy value, it will be fixed when we call '_update_ldap_group_permission'
+        'isProtected': ['FALSE']  # Dummy value, it will be fixed when we call '_update_ldap_group_permission'
-        'isProtected': ['FALSE'] # Dummy value, it will be fixed when we call '_update_ldap_group_permission'
     }
 
     if allowed is not None:
@@ -571,40 +575,35 @@ def _update_ldap_group_permission(permission, allowed,
     from yunohost.utils.ldap import _get_ldap_interface
     ldap = _get_ldap_interface()
 
-    # Fetch currently allowed groups for this permission
     existing_permission = user_permission_list(full=True, full_path=False)["permissions"][permission]
 
-    if allowed is None:
+    update = {}
-        allowed = existing_permission['allowed']
 
-    if label is None:
+    if allowed is not None:
-        label = existing_permission["label"]
+        allowed = [allowed] if not isinstance(allowed, list) else allowed
+        # Guarantee uniqueness of values in allowed, which would otherwise make ldap.update angry.
+        allowed = set(allowed)
+        update['groupPermission'] = ['cn=' + g + ',ou=groups,dc=yunohost,dc=org' for g in allowed]
 
-    if show_tile is None:
+    if label is not None:
-        show_tile = existing_permission["show_tile"]
+        update["label"] = [str(label)]
-    elif show_tile is True:
-        if not existing_permission['url']:
-            logger.warning(m18n.n('show_tile_cant_be_enabled_for_url_not_defined', permission=permission))
-            show_tile = False
-        elif existing_permission['url'].startswith('re:'):
-            logger.warning(m18n.n('show_tile_cant_be_enabled_for_regex', permission=permission))
-            show_tile = False
 
-    if protected is None:
+    if protected is not None:
-        protected = existing_permission["protected"]
+        update["isProtected"] = [str(protected).upper()]
 
-    allowed = [allowed] if not isinstance(allowed, list) else allowed
+    if show_tile is not None:
 
-    # Guarantee uniqueness of values in allowed, which would otherwise make ldap.update angry.
+        if show_tile is True:
-    allowed = set(allowed)
+            if not existing_permission['url']:
+                logger.warning(m18n.n('show_tile_cant_be_enabled_for_url_not_defined', permission=permission))
+                show_tile = False
+            elif existing_permission['url'].startswith('re:'):
+                logger.warning(m18n.n('show_tile_cant_be_enabled_for_regex', permission=permission))
+                show_tile = False
+        update["showTile"] = [str(show_tile).upper()]
 
     try:
-        ldap.update('cn=%s,ou=permission' % permission,
+        ldap.update('cn=%s,ou=permission' % permission, update)
-                    {'groupPermission': ['cn=' + g + ',ou=groups,dc=yunohost,dc=org' for g in allowed],
-                     'label': [str(label)] if label != "" else [],
-                     'showTile': [str(show_tile).upper()],
-                     'isProtected': [str(protected).upper()]
-                     })
     except Exception as e:
         raise YunohostError('permission_update_failed', permission=permission, error=e)
 

src/yunohost/tests/test_permission.py

@@ -292,10 +292,13 @@ def can_access_webpage(webpath, logged_as=None):
 def test_permission_list():
     res = user_permission_list(full=True)['permissions']
 
-    assert "wiki.main" in res
-    assert "blog.main" in res
     assert "mail.main" in res
     assert "xmpp.main" in res
+
+    assert "wiki.main" in res
+    assert "blog.main" in res
+    assert "blog.api" in res
+
     assert res['wiki.main']['allowed'] == ["all_users"]
     assert res['blog.main']['allowed'] == ["alice"]
     assert res['blog.api']['allowed'] == ["visitors"]
@@ -385,26 +388,26 @@ def test_permission_create_with_tile_management_with_main_default_value(mocker):
     assert res['site.main']['show_tile'] == True
 
 def test_permission_create_with_tile_management_with_not_main_default_value(mocker):
-    with message(mocker, "permission_created", permission="site.api"):
+    with message(mocker, "permission_created", permission="wiki.api"):
-        _permission_create_with_dummy_app("site.api", allowed=["all_users"], show_tile=True, url="/",
+        _permission_create_with_dummy_app("wiki.api", allowed=["all_users"], show_tile=True, url="/",
                                           domain=maindomain, path='/site')
 
     res = user_permission_list(full=True)['permissions']
-    assert "site.api" in res
+    assert "wiki.api" in res
-    assert res['site.api']['label'] == "Site (api)"
+    assert res['wiki.api']['label'] == "Wiki (api)"
-    assert res['site.api']['show_tile'] == True
+    assert res['wiki.api']['show_tile'] == True
 
 
 def test_permission_create_with_urls_management_without_url(mocker):
-    with message(mocker, "permission_created", permission="site.api"):
+    with message(mocker, "permission_created", permission="wiki.api"):
-        _permission_create_with_dummy_app("site.api", allowed=["all_users"],
+        _permission_create_with_dummy_app("wiki.api", allowed=["all_users"],
                                           domain=maindomain, path='/site')
 
     res = user_permission_list(full=True)['permissions']
-    assert "site.api" in res
+    assert "wiki.api" in res
-    assert res['site.api']['url'] == None
+    assert res['wiki.api']['url'] == None
-    assert res['site.api']['additional_urls'] == []
+    assert res['wiki.api']['additional_urls'] == []
-    assert res['site.api']['auth_header'] == True
+    assert res['wiki.api']['auth_header'] == True
 
 
 def test_permission_create_with_urls_management_simple_domain(mocker):