From 638b405bd1e1e2e56f7c1ece7e9cc6e8b8853ba5 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Fri, 7 Feb 2025 02:03:10 +0100 Subject: [PATCH] app/permissions: implement ability to custom app tile logo, description, order, hide from public app list view --- share/actionsmap.yml | 21 +++- src/app.py | 36 +++--- .../0033_rework_permission_infos.py | 2 + src/permission.py | 107 ++++++++++++------ src/portal.py | 20 +++- src/tests/test_app_resources.py | 5 +- src/tests/test_permission.py | 32 +++--- src/user.py | 68 +++++++++-- src/utils/resources.py | 18 +-- 9 files changed, 212 insertions(+), 97 deletions(-) diff --git a/share/actionsmap.yml b/share/actionsmap.yml index df0244267f..03ac36b1af 100755 --- a/share/actionsmap.yml +++ b/share/actionsmap.yml @@ -365,10 +365,27 @@ user: help: Permission to manage (e.g. mail or nextcloud or wordpress.editors) (use "yunohost user permission list" and "yunohost user permission -f" to see all the current permissions) -l: full: --label - help: Label for this permission. This label will be shown on the SSO and in the admin + help: Custom label for this app / permission -s: full: --show_tile - help: Define if a tile will be shown in the SSO + help: Define if a tile will be shown in the user portal + choices: + - 'True' + - 'False' + -L: + full: --logo + help: File to use as logo for this app / permission. Only PNG are supported. + type: argparse.FileType('rb') + -d: + full: --description + help: Custom description for this app / permission + -o: + full: --order + help: Order number to be used when displaying the tiles in the user portal. Default is 100 so set this to any low value for the tile to appear first, or higher value to appear last. + type: int + -H: + full: --hide_from_public + help: Mark the tile as to be hidden from the 'public app list' (if enabled). Useful for apps such as Nextcloud that need to be exposed to be publicly exposed for desktop/mobile client to be able to connect to, but not meant to be listed for visitors. choices: - 'True' - 'False' diff --git a/src/app.py b/src/app.py index 442f151754..b48c1314c0 100644 --- a/src/app.py +++ b/src/app.py @@ -132,10 +132,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) settings = _get_app_settings(app) + main_perm = settings.get("_permissions", {}).get("main", {}) ret = { - "description": _value_for_locale(local_manifest["description"]), - "name": settings.get("label", local_manifest["name"]), + "description": main_perm.get("description") or _value_for_locale(local_manifest["description"]), + "name": main_perm.get("label") or settings.get("label") or local_manifest["name"], "version": local_manifest.get("version", "-"), } @@ -153,7 +154,7 @@ def app_info(app, full=False, upgradable=False): ret["logo"] = ( app if os.path.exists(f"{APPS_CATALOG_LOGOS}/{app}.png") - else from_catalog.get("logo_hash") + else (main_perm.get("logo_hash") or from_catalog.get("logo_hash")) ) ret["upgradable"] = _app_upgradable({**ret, "from_catalog": from_catalog}) @@ -1618,24 +1619,23 @@ def app_register_url(app, domain, path): _sync_permissions_with_ldap() -def app_ssowatconf(): +def app_ssowatconf() -> None: """ Regenerate SSOwat configuration file - """ from yunohost.domain import ( _get_domain_portal_dict, _get_raw_domain_settings, domain_list, ) - from yunohost.permission import user_permission_list + from yunohost.permission import AppPermInfos, user_permission_list domain_portal_dict = _get_domain_portal_dict() domains = domain_list()["domains"] portal_domains = domain_list(exclude_subdomains=True)["domains"] - all_permissions = user_permission_list( + all_permissions: dict[str, AppPermInfos] = user_permission_list( full=True, ignore_system_perms=True, absolute_urls=True )["permissions"] @@ -1675,7 +1675,9 @@ def app_ssowatconf(): redirected_urls[domain + "/"] = domain_portal_dict[domain] # Will organize apps by portal domain - portal_domains_apps = {domain: {} for domain in portal_domains} + portal_domains_apps: dict[str, dict[str, dict]] = { + domain: {} for domain in portal_domains + } # This check is to prevent an issue during postinstall if the catalog cant # be initialized (because of offline postinstall) and it's not a big deal @@ -1765,16 +1767,20 @@ def app_ssowatconf(): "users": perm_info["corresponding_users"], "public": "visitors" in perm_info["allowed"], "url": uris[0], - "description": local_manifest["description"], + "description": perm_info.get("description") or local_manifest["description"], + "order": perm_info.get("order", 100), } - # FIXME : find a smarter way to get this info ? (in the settings maybe..) - # Also ideally we should not rely on the webadmin route for this, maybe expose these through a different route in nginx idk - # Also related to "people will want to customize those.." - app_catalog_info = apps_catalog.get(app_id.split("__")[0]) - if app_catalog_info and "logo_hash" in app_catalog_info: + if perm_info.get("hide_from_public"): + app_portal_info["hide_from_public"] = True + + # Logo may be customized via the perm setting, otherwise we use the default logo that we fetch from the catalog infos + app_base_id = app_id.split("__")[0] + # Use the perm logo, or the main-perm logo, or the default logo from catalog + logo_hash = perm_info.get("logo_hash") or all_permissions[f"{app_id}.main"].get("logo_hash") or apps_catalog.get(app_base_id, {}).get("logo_hash") + if logo_hash: app_portal_info["logo"] = ( - f"/yunohost/sso/applogos/{app_catalog_info['logo_hash']}.png" + f"/yunohost/sso/applogos/{logo_hash}.png" ) portal_domains_apps[app_portal_domain][perm_name] = app_portal_info diff --git a/src/migrations/0033_rework_permission_infos.py b/src/migrations/0033_rework_permission_infos.py index c59ccdf6ec..9bbc39b304 100644 --- a/src/migrations/0033_rework_permission_infos.py +++ b/src/migrations/0033_rework_permission_infos.py @@ -40,6 +40,8 @@ def run(self, *args): def run_after_system_restore(self): + regen_conf(["slapd"], force=True) + _, permission_system = self.read_legacy_permissions() _set_system_perms(permission_system) diff --git a/src/permission.py b/src/permission.py index 311f864b90..7c1917ec7d 100644 --- a/src/permission.py +++ b/src/permission.py @@ -24,7 +24,7 @@ import re import os from logging import getLogger -from typing import TYPE_CHECKING, Literal, TypedDict, NotRequired, cast +from typing import TYPE_CHECKING, BinaryIO, Literal, TypedDict, NotRequired, cast from moulinette import m18n from moulinette.utils.filesystem import read_yaml, write_to_yaml @@ -56,6 +56,10 @@ class AppPermInfos(SystemPermInfos): auth_header: bool protected: bool show_tile: bool | None + hide_from_public: NotRequired[bool] + logo_hash: NotRequired[str] + description: NotRequired[str] + order: NotRequired[int] PermInfos = AppPermInfos | SystemPermInfos @@ -81,10 +85,6 @@ def user_permission_list( from yunohost.app import _installed_apps, _get_app_settings from yunohost.user import user_group_list - map_group_to_users = { - g: infos["members"] for g, infos in user_group_list()["groups"].items() - } - # Parse / organize information to be outputed filter_ = apps if filter_: @@ -97,13 +97,15 @@ def user_permission_list( settings = _get_app_settings(app) subperms = settings.get("_permissions", {}) - default_app_label = settings.get("label") or app.title() if "main" not in subperms: subperms["main"] = {} + + app_label = subperms["main"].get("label") or settings.get("label") or app.title() + for subperm, infos in subperms.items(): name = f"{app}.{subperm}" perm: AppPermInfos = { - "label": default_app_label, + "label": "", "url": None, "additional_urls": [], "auth_header": True, @@ -113,7 +115,12 @@ def user_permission_list( } perm.update(infos) if subperm != "main": - perm["label"] += " (" + settings.get("label", subperm) + ")" + # Redefine the subperm label to : () + subperm_label = (perm["label"] or subperm) + perm["label"] = f"{app_label} ({subperm_label})" + elif not perm["label"]: + perm["label"] = app_label + if perm["show_tile"] is None and perm["url"] is not None: perm["show_tile"] = True @@ -147,18 +154,27 @@ def user_permission_list( permissions[f"{name}.main"] = system_perm_conf[name] if full: - for permission in permissions.values(): - permission["corresponding_users"] = set() - for group in permission["allowed"]: + map_group_to_users = { + g: infos["members"] for g, infos in user_group_list()["groups"].items() + } + for infos in permissions.values(): + infos["corresponding_users"] = set() + for group in infos["allowed"]: # FIXME: somewhere we may want to have some sort of garbage collection # to automatically remove user/groups from the "allowed" info when they - # somehow disappared from the system (for example this may happen when + # somehow disappeared from the system (for example this may happen when # restoring an app on which not all the user/group exist) users_in_group = set(map_group_to_users.get(group, [])) - permission["corresponding_users"] |= users_in_group # type: ignore - permission["corresponding_users"] = list( - sorted(permission["corresponding_users"]) + infos["corresponding_users"] |= users_in_group + infos["corresponding_users"] = list( + sorted(infos["corresponding_users"]) ) + else: + # Keep the output concise when used without --full, meant to not bloat CLI + for infos in permissions.values(): + for key in ["additional_urls", "auth_header", "logo_hash", "order", "protected", "show_tile"]: + if key in infos: + del infos[key] return {"permissions": permissions} @@ -268,13 +284,6 @@ def user_permission_update( if "visitors" not in new_allowed_groups or len(new_allowed_groups) >= 3: logger.warning(m18n.n("permission_currently_allowed_for_all_users")) - # Note that we can get this argument as string if we it come from the CLI - if isinstance(show_tile, str): - if show_tile.lower() == "true": - show_tile = True - else: - show_tile = False - if ( existing_permission.get("url") and existing_permission["url"].startswith("re:") # type: ignore @@ -392,14 +401,6 @@ def permission_create( if "." not in permission: permission = permission + ".main" - # Get random GID - all_gid = {x.gr_gid for x in grp.getgrall()} - - uid_guid_found = False - while not uid_guid_found: - gid = str(random.randint(200, 99999)) - uid_guid_found = gid not in all_gid - app, subperm = permission.split(".") if allowed is not None: @@ -560,7 +561,7 @@ def permission_url( def permission_delete( permission: str, force: bool = False, sync_perm: bool = True ) -> None: - from yunohost.app import app_setting, _is_installed, app_ssowatconf + from yunohost.app import app_setting, _assert_is_installed, app_ssowatconf # By default, manipulate main permission if "." not in permission: @@ -571,12 +572,16 @@ def permission_delete( app, subperm = permission.split(".") - if _is_installed(app): - # Actually delete the permission - perm_settings = app_setting(app, "_permissions") or {} - if subperm in perm_settings: - del perm_settings[subperm] - app_setting(app, "_permissions", perm_settings) + if app in SYSTEM_PERMS: + raise YunohostValidationError(f"Cannot delete system permission {permission}", raw_msg=True) + + _assert_is_installed(app) + + # Actually delete the permission + perm_settings = app_setting(app, "_permissions") or {} + if subperm in perm_settings: + del perm_settings[subperm] + app_setting(app, "_permissions", perm_settings) if sync_perm: _sync_permissions_with_ldap() @@ -699,6 +704,10 @@ def _update_app_permission_setting( show_tile: bool | None = None, protected: bool | None = None, allowed: str | list[str] | None = None, + logo: BinaryIO | None = None, + description: str | None = None, + hide_from_public: bool | None = None, + order: int | None = None, ) -> None: from yunohost.app import app_setting @@ -711,6 +720,30 @@ def _update_app_permission_setting( if label is not None: update_settings["label"] = str(label) + if description is not None: + update_settings["description"] = description + + if hide_from_public is not None: + update_settings["hide_from_public"] = hide_from_public + + if order is not None: + update_settings["order"] = order + + if logo is not None: + + from yunohost.app import APPS_CATALOG_LOGOS + import hashlib + + logo_content = logo.read() + if not logo_content.startswith(b"\x89PNG\r\n\x1a\n"): + raise YunohostValidationError("The provided logo file doesn't seem to be a PNG file. Only PNG logos are supported.", raw_msg=True) + + logo_hash = hashlib.sha256(logo_content).hexdigest() + with open(f"{APPS_CATALOG_LOGOS}/{logo_hash}.png", "wb") as f: + f.write(logo_content) + + update_settings["logo_hash"] = logo_hash + if protected is not None: update_settings["protected"] = protected diff --git a/src/portal.py b/src/portal.py index d695b623c1..2672b5535a 100644 --- a/src/portal.py +++ b/src/portal.py @@ -55,7 +55,7 @@ def _get_user_infos( def _get_portal_settings( domain: Union[str, None] = None, username: Union[str, None] = None -): +) -> dict[str, Any]: """ Returns domain's portal settings which are a combo of domain's portal config panel options and the list of apps availables on this domain computed by `app.app_ssowatconf()`. @@ -101,19 +101,27 @@ def _get_portal_settings( if username: # Add user allowed or public apps settings["apps"] = { - name: app - for name, app in apps.items() - if username in app["users"] or app["public"] + app: infos + for app, infos in apps.items() + if username in infos["users"] or infos["public"] } elif settings["public"]: # Add public apps (e.g. with "visitors" in group permission) - settings["apps"] = {name: app for name, app in apps.items() if app["public"]} + settings["apps"] = { + app: infos + for app, infos in apps.items() + if infos["public"] and not infos.get("hide_from_public") + } + + # Sort dictionnary according to the "order" info + settings["apps"] = dict(sorted([(app, infos) for app, infos in settings["apps"].items()], key=lambda v: (v[1].get("order", 100), v[0]))) return settings def portal_public(): - """Get public settings + """ + Get public settings If the portal is set as public, it will include the list of public apps """ diff --git a/src/tests/test_app_resources.py b/src/tests/test_app_resources.py index afa0dfab4c..2d8b4bd634 100644 --- a/src/tests/test_app_resources.py +++ b/src/tests/test_app_resources.py @@ -371,12 +371,13 @@ def test_resource_permissions(): "main": { "url": "/", "allowed": "visitors", - # TODO: test protected? }, } res = user_permission_list(full=True)["permissions"] - assert not any(key.startswith("testapp.") for key in res) + # Nowadays there's always an implicit "main" perm but with default stuff such as empty url + assert res["testapp.main"]["url"] is None + assert res["testapp.main"]["allowed"] == [] r(conf, "testapp", manager).provision_or_update() diff --git a/src/tests/test_permission.py b/src/tests/test_permission.py index e9d2670c4e..f8256389e9 100644 --- a/src/tests/test_permission.py +++ b/src/tests/test_permission.py @@ -53,6 +53,7 @@ user_group_list, user_list, ) +from yunohost.user import user_permission_update as user_permission_update_from_cli from .conftest import get_test_apps_dir, message, raiseYunohostError @@ -571,18 +572,21 @@ def test_permission_create_with_urls_management_multiple_domain(): def test_permission_delete(): - with message("permission_deleted", permission="wiki.main"): - permission_delete("wiki.main", force=True) - - res = user_permission_list()["permissions"] - assert "wiki.main" not in res - with message("permission_deleted", permission="blog.api"): permission_delete("blog.api", force=False) res = user_permission_list()["permissions"] assert "blog.api" not in res +def test_permission_delete_cant_really_delete_main(): + with message("permission_deleted", permission="wiki.main"): + permission_delete("wiki.main", force=True) + + # A "main" perm will always be injected so it is still outputed by user_permission_list + res = user_permission_list()["permissions"] + assert "wiki.main" in res + + # # Error on create - remove function @@ -590,12 +594,12 @@ def test_permission_delete(): def test_permission_create_already_existing(mocker): - with raiseYunohostError(mocker, "permission_already_exist"): - permission_create("wiki.main") + # This doesn't raise an exception anymoar by design + permission_create("wiki.main") -def test_permission_delete_doesnt_existing(mocker): - with raiseYunohostError(mocker, "permission_not_found"): +def test_permission_delete_for_unknown_app(mocker): + with raiseYunohostError(mocker, "app_not_installed"): permission_delete("doesnt.exist", force=True) res = user_permission_list()["permissions"] @@ -695,14 +699,14 @@ def test_permission_switch_show_tile(): # Note that from the actionmap the value is passed as string, not as bool # Try with lowercase with message("permission_updated", permission="wiki.main"): - user_permission_update("wiki.main", show_tile="false") + user_permission_update_from_cli("wiki.main", show_tile="false") res = user_permission_list(full=True)["permissions"] assert res["wiki.main"]["show_tile"] is False # Try with uppercase with message("permission_updated", permission="wiki.main"): - user_permission_update("wiki.main", show_tile="TRUE") + user_permission_update_from_cli("wiki.main", show_tile="TRUE") res = user_permission_list(full=True)["permissions"] assert res["wiki.main"]["show_tile"] is True @@ -711,7 +715,7 @@ def test_permission_switch_show_tile(): def test_permission_switch_show_tile_with_same_value(): # Note that from the actionmap the value is passed as string, not as bool with message("permission_updated", permission="wiki.main"): - user_permission_update("wiki.main", show_tile="True") + user_permission_update_from_cli("wiki.main", show_tile="True") res = user_permission_list(full=True)["permissions"] assert res["wiki.main"]["show_tile"] is True @@ -732,7 +736,7 @@ def test_permission_add_group_that_doesnt_exist(mocker): def test_permission_update_permission_that_doesnt_exist(mocker): - with raiseYunohostError(mocker, "permission_not_found"): + with raiseYunohostError(mocker, "app_not_installed"): user_permission_update("doesnt.exist", add="alice") diff --git a/src/user.py b/src/user.py index e73e7850fb..dc8036468a 100644 --- a/src/user.py +++ b/src/user.py @@ -26,7 +26,7 @@ import re import subprocess from logging import getLogger -from typing import TYPE_CHECKING, Any, Callable, TextIO, Union, cast, Literal +from typing import TYPE_CHECKING, Any, Callable, TextIO, BinaryIO, Union, cast, Literal from moulinette import Moulinette, m18n from moulinette.utils.process import check_output @@ -1048,6 +1048,18 @@ def user_group_list( _ldap_path_extract(p, "uid") for p in infos.get("member", []) ] + if full: + for group in groups: + groups[group]["permissions"] = [] + + from yunohost.permission import user_permission_list + + perms = user_permission_list(full=False)["permissions"] + for perm, infos in perms.items(): + for group in infos["allowed"]: + if group in groups: + groups[group]["permissions"].append(perm) + return {"groups": groups} @@ -1413,7 +1425,7 @@ def user_group_info(groupname: str) -> dict[str, Any]: result = ldap.search( "ou=groups", "cn=" + groupname, - ["cn", "member", "permission", "mail"], + ["cn", "member", "mail"], ) if not result: @@ -1494,16 +1506,51 @@ def user_permission_list( @is_unit_operation(flash=True) def user_permission_update( permission: str, - label: Optional[str] = None, - show_tile: Optional[bool] = None, - sync_perm: bool = True, -) -> "PermInfos": - from yunohost.permission import user_permission_update + label: str | None = None, + show_tile: bool | None = None, + logo: BinaryIO | None = None, + description: str | None = None, + hide_from_public: bool | None = None, + order: int | None = None, +) -> dict[str, Any]: - return user_permission_update( - permission, label=label, show_tile=show_tile, sync_perm=sync_perm + from yunohost.app import _assert_is_installed, app_ssowatconf, app_setting + from yunohost.permission import _update_app_permission_setting + + # By default, manipulate main permission + if "." not in permission: + permission = permission + ".main" + + app, permname = permission.split(".", 1) + _assert_is_installed(app) + + if permname not in (app_setting(app, "_permissions") or {}): + raise YunohostValidationError( + f"Unknown permission {permname} for app {app}", raw_msg=True + ) + + # We get these from CLI as string (because we want to be able to differentiate between True, False and "unspecified" = "do not change the value" + if isinstance(show_tile, str): + show_tile = True if show_tile.lower() == "true" else False + if isinstance(hide_from_public, str): + hide_from_public = True if hide_from_public.lower() == "true" else False + + _update_app_permission_setting( + permission=permission, + label=label, + show_tile=show_tile, + logo=logo, + description=description, + hide_from_public=hide_from_public, + order=order, ) + app_ssowatconf() + + logger.success(m18n.n("permission_updated", permission=permission)) + + return (app_setting(app, "_permissions") or {}).get(permname, "") + @is_unit_operation(flash=True) def user_permission_add( @@ -1554,16 +1601,19 @@ def user_permission_ldapsync() -> None: def user_ssh_list_keys(username: str) -> dict[str, dict[str, str]]: from yunohost.ssh import user_ssh_list_keys + return user_ssh_list_keys(username) def user_ssh_add_key(username: str, key: str, comment: str | None = None) -> None: from yunohost.ssh import user_ssh_add_key + return user_ssh_add_key(username, key, comment) def user_ssh_remove_key(username: str, key: str) -> None: from yunohost.ssh import user_ssh_remove_key + return user_ssh_remove_key(username, key) diff --git a/src/utils/resources.py b/src/utils/resources.py index ed3fc01de3..22e768bcae 100644 --- a/src/utils/resources.py +++ b/src/utils/resources.py @@ -678,7 +678,6 @@ def provision_or_update(self, context: Dict = {}): permission_delete, _sync_permissions_with_ldap, permission_url, - user_permission_list, user_permission_update, ) @@ -695,16 +694,14 @@ def provision_or_update(self, context: Dict = {}): ): self.set_setting("path", "/") - existing_perms = user_permission_list(apps=[self.app])[ - "permissions" - ] + existing_perms = list((self.get_setting("_permissions") or {}).keys()) for perm in existing_perms: - if perm.split(".")[1] not in self.permissions.keys(): - permission_delete(perm, force=True, sync_perm=False) + if perm not in self.permissions.keys(): + permission_delete(f"{self.app}.{perm}", force=True, sync_perm=False) for perm, infos in self.permissions.items(): perm_id = f"{self.app}.{perm}" - if perm_id not in existing_perms: + if perm not in existing_perms: # Use the 'allowed' key from the manifest, # or use the 'init_{perm}_permission' from the install questions # which is temporarily saved as a setting as an ugly hack to pass the info to this piece of code... @@ -749,14 +746,11 @@ def deprovision(self, context: Dict = {}): from yunohost.permission import ( permission_delete, _sync_permissions_with_ldap, - user_permission_list, ) - existing_perms = user_permission_list(apps=[self.app])[ - "permissions" - ] + existing_perms = list((self.get_setting("_permissions") or {}).keys()) for perm in existing_perms: - permission_delete(perm, force=True, sync_perm=False) + permission_delete(f"{self.app}.{perm}", force=True, sync_perm=False) _sync_permissions_with_ldap()