Yolorework the label handling of permissions

src/app.py

@@ -130,15 +130,11 @@ def app_info(app, full=False, upgradable=False):
 
     setting_path = os.path.join(APPS_SETTING_PATH, app)
     local_manifest = _get_manifest_of_app(setting_path)
-    permissions = user_permission_list(full=True, absolute_urls=True, apps=[app])[
-        "permissions"
-    ]
-
     settings = _get_app_settings(app)
 
     ret = {
         "description": _value_for_locale(local_manifest["description"]),
-        "name": permissions.get(app + ".main", {}).get("label", local_manifest["name"]),
+        "name": settings.get("label", local_manifest["name"]),
         "version": local_manifest.get("version", "-"),
     }
 
@@ -246,12 +242,13 @@ def app_info(app, full=False, upgradable=False):
         and local_manifest["resources"].get("data_dir") is not None
     )
 
-    ret["permissions"] = permissions
-    ret["label"] = permissions.get(app + ".main", {}).get("label")
+    ret["permissions"] = user_permission_list(full=True, absolute_urls=True, apps=[app])[
+        "permissions"
+    ]
+
+    # FIXME : this is the same stuff as "name" ... maybe we should get rid of "name" ?
+    ret["label"] = settings.get("label", local_manifest["name"])
 
-    if not ret["label"]:
-        logger.debug(f"Failed to get label for app {app}, maybe it is not a webapp?")
-        ret["label"] = local_manifest["name"]
     return ret
 
 
@@ -1148,13 +1145,20 @@ def app_install(
         shutil.rmtree(app_setting_path)
     os.makedirs(app_setting_path)
 
+    # Hotfix for bug in the webadmin while we fix the actual issue :D
+    if label == "undefined":
+        label = None
+
     # Set initial app settings
     app_settings = {
         "id": app_instance_name,
         "install_time": int(time.time()),
         "current_revision": manifest.get("remote", {}).get("revision", "?"),
     }
 
+    if label:
+        app_settings["label"] = label
+
     # If packaging_format v2+, save all install options as settings
     if packaging_format >= 2:
         for option in options:
@@ -1179,15 +1183,6 @@ def app_install(
                 recursive=True,
             )
 
-    # Hotfix for bug in the webadmin while we fix the actual issue :D
-    if label == "undefined":
-        label = None
-
-    # Override manifest name by given label
-    # This info is also later picked-up by the 'permission' resource initialization
-    if label:
-        manifest["name"] = label
-
     if packaging_format >= 2:
         from yunohost.utils.resources import AppResourceManager
 
@@ -1209,7 +1204,6 @@ def app_install(
         permission_create(
             app_instance_name + ".main",
             allowed=["all_users"],
-            label=manifest["name"],
             show_tile=False,
             protected=False,
         )
@@ -1812,19 +1806,14 @@ def app_ssowatconf():
 
 
 def app_change_label(app, new_label):
-    from yunohost.permission import user_permission_update
 
     installed = _is_installed(app)
     if not installed:
         raise YunohostValidationError(
             "app_not_installed", app=app, all_apps=_get_all_installed_apps_id()
         )
-    logger.warning(m18n.n("app_label_deprecated"))
-    user_permission_update(app + ".main", label=new_label)
-
 
-# actions todo list:
-# * docstring
+    app_setting(app, "label", new_label)
 
 
 def app_action_list(app):

src/backup.py

@@ -1295,19 +1295,14 @@ def _restore_system(self):
 
         # Restore permission for apps installed
         for permission_name, permission_infos in old_apps_permission.items():
-            app_name, perm_name = permission_name.split(".")
+            app_name, _ = permission_name.split(".")
             if _is_installed(app_name):
                 permission_create(
                     permission_name,
                     allowed=permission_infos["allowed"],
                     url=permission_infos["url"],
                     additional_urls=permission_infos["additional_urls"],
                     auth_header=permission_infos["auth_header"],
-                    label=(
-                        permission_infos["label"]
-                        if perm_name == "main"
-                        else permission_infos["sublabel"]
-                    ),
                     show_tile=permission_infos["show_tile"],
                     protected=permission_infos["protected"],
                     sync_perm=False,
@@ -1432,18 +1427,12 @@ def copytree(src, dst, symlinks=False, ignore=None):
                         g for g in permission_infos["allowed"] if g in existing_groups
                     ]
 
-                perm_name = permission_name.split(".")[1]
                 permission_create(
                     permission_name,
                     allowed=should_be_allowed,
                     url=permission_infos.get("url"),
                     additional_urls=permission_infos.get("additional_urls"),
                     auth_header=permission_infos.get("auth_header"),
-                    label=(
-                        permission_infos.get("label")
-                        if perm_name == "main"
-                        else permission_infos.get("sublabel")
-                    ),
                     show_tile=permission_infos.get("show_tile", True),
                     protected=permission_infos.get("protected", False),
                     sync_perm=False,

src/permission.py

@@ -28,7 +28,11 @@
 
 logger = getLogger("yunohost.user")
 
-SYSTEM_PERMS = ["mail", "sftp", "ssh"]
+SYSTEM_PERMS = {
+    "mail": "Email",
+    "sftp": "SFTP",
+    "ssh": "SSH",
+}
 
 #
 #
@@ -93,6 +97,11 @@ def user_permission_list(
             }
             perm_settings = (app_setting(app, "_permissions") or {}).get(subperm, {})
             perm.update(perm_settings)
+
+            perm["label"] = app_setting(app, "label") or app.title()
+            if subperm != "main":
+                perm["label"] += " (" + perm_settings.get("label", subperm) + ")"
+
             if perm["show_tile"] is None and perm["url"] is not None:
                 perm["show_tile"] = True
 
@@ -105,6 +114,10 @@ def user_permission_list(
                     _get_absolute_url(url, app_base_path)
                     for url in perm["additional_urls"]
                 ]
+        elif full and app in SYSTEM_PERMS:
+            perm["label"] = SYSTEM_PERMS[app]
+            if subperm != "main":
+                perm["label"] += f" ({subperm})"
 
         perm["allowed"] = [
             _ldap_path_extract(p, "cn") for p in infos.get("groupPermission", [])
@@ -116,23 +129,6 @@ def user_permission_list(
 
         permissions[name] = perm
 
-    # Make sure labels for sub-permissions are the form " Applabel (Sublabel) "
-    if full:
-        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(
-                    f"Uhoh, unknown permission {main_perm_name} ? (Maybe we're in the process or deleting the perm for this app...)"
-                )
-                continue
-            main_perm_label = permissions[main_perm_name]["label"]
-            infos["sublabel"] = infos["label"]
-            label_ = infos["label"]
-            infos["label"] = f"{main_perm_label} ({label_})"
-
     if short:
         permissions = list(permissions.keys())
 
@@ -358,7 +354,6 @@ def permission_create(
     url=None,
     additional_urls=None,
     auth_header=True,
-    label=None,
     show_tile=False,
     protected=False,
     sync_perm=True,
@@ -372,7 +367,6 @@ def permission_create(
         url             -- (optional) URL for which access will be allowed/forbidden
         additional_urls -- (optional) List of additional URL for which access will be allowed/forbidden
         auth_header     -- (optional) Define for the URL of this permission, if SSOwat pass the authentication header to the application
-        label           -- (optional) Define a name for the permission. This label will be shown on the SSO and in the admin. Default is "permission name"
         show_tile       -- (optional) Define if a tile will be shown in the SSO
         protected       -- (optional) Define if the permission can be added/removed to the visitor group
 
@@ -437,8 +431,6 @@ def permission_create(
             "permission_creation_failed", permission=permission, error=e
         )
 
-    label = str(label) if label else (subperm if subperm != "main" else app.title())
-
     try:
         permission_url(
             permission,
@@ -451,7 +443,6 @@ def permission_create(
         new_permission = _update_ldap_group_permission(
             permission=permission,
             allowed=allowed,
-            label=label,
             show_tile=show_tile,
             protected=protected,
             sync_perm=sync_perm,
@@ -760,16 +751,22 @@ def _update_ldap_group_permission(
                 update_settings["show_tile"] = False
 
     if app not in SYSTEM_PERMS:
+
+        if "label" in update_settings and sub_permission == "main":
+            label = update_settings.pop("label")
+            app_setting(app, "label", label)
+
         perm_settings = app_setting(app, "_permissions") or {}
         if sub_permission not in perm_settings:
             perm_settings[sub_permission] = {}
         perm_settings[sub_permission].update(update_settings)
         app_setting(app, "_permissions", perm_settings)
 
-    try:
-        ldap.update(f"cn={permission},ou=permission", update_ldap)
-    except Exception as e:
-        raise YunohostError("permission_update_failed", permission=permission, error=e)
+    if update_ldap:
+        try:
+            ldap.update(f"cn={permission},ou=permission", update_ldap)
+        except Exception as e:
+            raise YunohostError("permission_update_failed", permission=permission, error=e)
 
     # Trigger permission sync if asked
 

src/tests/test_permission.py

@@ -53,7 +53,6 @@ def _permission_create_with_dummy_app(
     url=None,
     additional_urls=None,
     auth_header=True,
-    label=None,
     show_tile=False,
     protected=True,
     sync_perm=True,
@@ -88,7 +87,6 @@ def _permission_create_with_dummy_app(
         url=url,
         additional_urls=additional_urls,
         auth_header=auth_header,
-        label=label,
         show_tile=show_tile,
         protected=protected,
         sync_perm=sync_perm,
@@ -165,7 +163,6 @@ def new_getaddrinfo(*args):
         url="/",
         additional_urls=["/whatever", "/idontnow"],
         auth_header=False,
-        label="Wiki",
         show_tile=True,
         allowed=["all_users"],
         protected=False,
@@ -478,15 +475,14 @@ def test_permission_create_with_tile_management():
         _permission_create_with_dummy_app(
             "site.main",
             allowed=["all_users"],
-            label="The Site",
             show_tile=False,
             domain=maindomain,
             path="/site",
         )
 
     res = user_permission_list(full=True)["permissions"]
     assert "site.main" in res
-    assert res["site.main"]["label"] == "The Site"
+    assert res["site.main"]["label"] == "Site"
     assert res["site.main"]["show_tile"] is False
 
 
@@ -958,7 +954,6 @@ def test_show_tile_cant_be_enabled():
     _permission_create_with_dummy_app(
         permission="site.main",
         auth_header=False,
-        label="Site",
         show_tile=True,
         allowed=["all_users"],
         protected=False,
@@ -971,7 +966,6 @@ def test_show_tile_cant_be_enabled():
         permission="web.main",
         url="re:/[a-z]{3}/bla",
         auth_header=False,
-        label="Web",
         show_tile=True,
         allowed=["all_users"],
         protected=False,

src/utils/resources.py

@@ -146,7 +146,7 @@ class AppResource:
 
     def __init__(self, properties: Dict[str, Any], app: str, manager=None):
         self.app = app
-        self.manager = manager
+        self.workdir = manager.workdir
         properties = self.default_properties | properties
 
         # It's not guaranteed that this info will be defined, e.g. during unit tests, only small resource snippets are used, not proper manifests
@@ -257,11 +257,7 @@ def _run_script(self, action, script, env={}):
         )
         from yunohost.hook import hook_exec_with_script_debug_if_failure
 
-        workdir = (
-            self.manager.workdir
-            if self.manager and self.manager.workdir
-            else _make_tmp_workdir_for_app(app=self.app)
-        )
+        workdir = self.workdir or _make_tmp_workdir_for_app(app=self.app)
 
         env_ = _make_environment_for_app_script(
             self.app,
@@ -727,8 +723,6 @@ def provision_or_update(self, context: Dict = {}):
                 permission_create(
                     perm_id,
                     allowed=init_allowed,
-                    # This is why the ugly hack with self.manager exists >_>
-                    label=self.manager.wanted["name"] if perm == "main" else perm,
                     url=infos["url"],
                     additional_urls=infos["additional_urls"],
                     auth_header=infos["auth_header"],