From 0175053c7329824217bfe1d3c11b66f297b3acd0 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Sun, 1 Dec 2024 01:29:50 +0100 Subject: [PATCH] Yolorework the label handling of permissions --- src/app.py | 41 +++++++++++------------------ src/backup.py | 5 ---- src/permission.py | 51 +++++++++++++++++------------------- src/tests/test_permission.py | 8 +----- src/utils/resources.py | 10 ++----- 5 files changed, 42 insertions(+), 73 deletions(-) diff --git a/src/app.py b/src/app.py index 5c00a15bcd..9d29e4c4d0 100644 --- a/src/app.py +++ b/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,6 +1145,10 @@ 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, @@ -1155,6 +1156,9 @@ def app_install( "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): diff --git a/src/backup.py b/src/backup.py index 9fd3a266bb..e8aabe5e99 100644 --- a/src/backup.py +++ b/src/backup.py @@ -1439,11 +1439,6 @@ def copytree(src, dst, symlinks=False, ignore=None): 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, diff --git a/src/permission.py b/src/permission.py index 02f9127a76..c55597d673 100644 --- a/src/permission.py +++ b/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 diff --git a/src/tests/test_permission.py b/src/tests/test_permission.py index 302173b1ca..5ea6c50677 100644 --- a/src/tests/test_permission.py +++ b/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,7 +475,6 @@ 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", @@ -486,7 +482,7 @@ def test_permission_create_with_tile_management(): 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, diff --git a/src/utils/resources.py b/src/utils/resources.py index b4707e5076..82ff323eb8 100644 --- a/src/utils/resources.py +++ b/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 = self.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"],