From fe5ca2422208c6c8653ba33310ab8ca4260d546a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josu=C3=A9=20Tille?= Date: Thu, 2 Apr 2020 16:17:48 +0200 Subject: [PATCH 01/12] Migrate old skipped,unprotected,protected_uris and create permission instead --- data/helpers.d/setting | 31 +++++-- src/yunohost/app.py | 92 ------------------- src/yunohost/backup.py | 9 ++ .../0015_extends_permissions_features_1.py | 47 +++++++++- 4 files changed, 76 insertions(+), 103 deletions(-) diff --git a/data/helpers.d/setting b/data/helpers.d/setting index abf6ab3d4..7edeed588 100644 --- a/data/helpers.d/setting +++ b/data/helpers.d/setting @@ -1,5 +1,7 @@ #!/bin/bash +migrate_to_permission_deprecitated_warning="/!\\ Packagers! This app is still using the skipped/protected/unprotected_uris/regex settings which are now obsolete and deprecated... Instead, you should use the new helpers 'ynh_permission_{create,urls,update,delete}' and the 'visitors' group to initialize the public/private access. Check out the documentation at the bottom of yunohost.org/groups_and_permissions to learn how to use the new permission mechanism.\n" + # Get an application setting # # usage: ynh_app_setting_get --app=app --key=key @@ -66,7 +68,7 @@ ynh_app_setting_delete() { # ynh_app_setting() { - if [[ "$1" == "delete" ]] && [[ "$3" =~ ^(unprotected|skipped)_ ]] + if [[ "$1" == "delete" ]] && [[ "$3" =~ ^(unprotected|skipped)_ ]] then current_value=$(ynh_app_setting_get --app=$app --key=$3) fi @@ -89,8 +91,6 @@ else: elif action == "set": if key in ['redirected_urls', 'redirected_regex']: value = yaml.load(value) - if any(key.startswith(word+"_") for word in ["unprotected", "protected", "skipped"]): - sys.stderr.write("/!\\ Packagers! This app is still using the skipped/protected/unprotected_uris/regex settings which are now obsolete and deprecated... Instead, you should use the new helpers 'ynh_permission_{create,urls,update,delete}' and the 'visitors' group to initialize the public/private access. Check out the documentation at the bottom of yunohost.org/groups_and_permissions to learn how to use the new permission mechanism.\n") settings[key] = value else: raise ValueError("action should either be get, set or delete") @@ -102,12 +102,23 @@ EOF # We need this because app temporarily set the app as unprotected to configure it with curl... if [[ "$3" =~ ^(unprotected|skipped)_ ]] then - if [[ "$1" == "set" ]] && [[ "${4:-}" == "/" ]] + if [[ "$1" == "delete" ]] then - ynh_permission_update --permission "main" --add "visitors" - elif [[ "$1" == "delete" ]] && [[ "${current_value:-}" == "/" ]] && [[ -n "$(ynh_app_setting_get --app=$2 --key='is_public' )" ]] - then - ynh_permission_update --permission "main" --remove "visitors" + if [[ "${current_value:-}" == "/" ]] && [[ -n "$(ynh_app_setting_get --app=$2 --key='is_public' )" ]] + then + ynh_permission_update --permission "main" --remove "visitors" + else + if [ "$3" == "skipped_uris" ] && ynh_permission_exists --permission legacy_skipped_uris + then + ynh_permission_delete --permission legacy_skipped_uris + elif [ "$3" == "unprotected_uris" ] && ynh_permission_exists --permission legacy_unprotected_uris + then + ynh_permission_delete --permission legacy_unprotected_uris + elif [ "$3" == "protected_uris" ] && ynh_permission_exists --permission legacy_protected_uris + then + ynh_permission_delete --permission legacy_protected_uris + fi + fi fi fi } @@ -253,7 +264,7 @@ ynh_permission_create() { protected=",protected=False" fi fi - + yunohost tools shell -c "from yunohost.permission import permission_create; permission_create('$app.$permission' $url $additional_urls $auth_header $allowed $label $show_tile $protected , sync_perm=False)" } @@ -294,7 +305,7 @@ ynh_permission_exists() { # Redefine the url associated to a permission # -# usage: ynh_permission_url --permission "permission" [--url "url"] [--add_url "new-url" [ "other-new-url" ]] [--remove_url "old-url" [ "other-old-url"]] +# usage: ynh_permission_url --permission "permission" [--url "url"] [--add_url "new-url" [ "other-new-url" ]] [--remove_url "old-url" [ "other-old-url"]] # [--auth_header true|false][--clear_urls] # | arg: permission - the name for the permission (by default a permission named "main" is removed automatically when the app is removed) # | arg: url - (optional) URL for which access will be allowed/forbidden. diff --git a/src/yunohost/app.py b/src/yunohost/app.py index 038040524..9db9c9918 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -1245,98 +1245,6 @@ def app_ssowatconf(): app_settings = read_yaml(APPS_SETTING_PATH + app + '/settings.yml') - ## BEGIN Legacy part ## - - if 'domain' not in app_settings: - continue - if 'path' not in app_settings: - continue - - # This 'no_sso' settings sound redundant to not having $path defined .... - # At least from what I can see, all apps using it don't have a path defined ... - if 'no_sso' in app_settings: - continue - - domain = app_settings['domain'] - path = app_settings['path'].rstrip('/') - - def _sanitized_absolute_url(perm_url): - # Nominal case : url is relative to the app's path - if perm_url.startswith("/"): - perm_domain = domain - perm_path = path + perm_url.rstrip("/") - # Otherwise, the urls starts with a domain name, like domain.tld/foo/bar - # We want perm_domain = domain.tld and perm_path = "/foo/bar" - else: - perm_domain, perm_path = perm_url.split("/", 1) - perm_path = "/" + perm_path.rstrip("/") - - perm_path = perm_path if perm_path.strip() != "" else "/" - - return perm_domain + perm_path - - # Skipped - skipped_urls = [_sanitized_absolute_url(uri) for uri in _get_setting(app_settings, 'skipped_uris')] - skipped_urls += ['re:' + regex for regex in _get_setting(app_settings, 'skipped_regex')] - - # Legacy permission system using (un)protected_uris and _regex managed in app settings... - unprotected_urls = [_sanitized_absolute_url(uri) for uri in _get_setting(app_settings, 'unprotected_uris')] - protected_urls = [_sanitized_absolute_url(uri) for uri in _get_setting(app_settings, 'protected_uris')] - unprotected_urls += ['re:' + regex for regex in _get_setting(app_settings, 'unprotected_regex')] - protected_urls += ['re:' + regex for regex in _get_setting(app_settings, 'protected_regex')] - - if skipped_urls == [] and unprotected_urls == [] and protected_urls == []: - continue - - # Manage compatibility with old protected, unprotected, skipped urls !! - this_app_perms = {name: info for name, info in all_permissions.items() if name.startswith(app + ".")} - for perm_name, perm_info in this_app_perms.items(): - - # Ignore permissions for which there's no url defined - if not perm_info["url"]: - continue - - url = _sanitized_absolute_url(perm_info["url"]) - perm_info["url"] = url - if "visitors" in perm_info["allowed"]: - # Legacy stuff : we remove now protected-urls that might have been declared as unprotected earlier... - protected_urls = [u for u in protected_urls if u != url] - else: - # Legacy stuff : we remove now unprotected-urls / skipped-urls that might have been declared as protected earlier... - unprotected_urls = [u for u in unprotected_urls if u != url] - skipped_urls = [u for u in skipped_urls if u != url] - - # Create special permission for legacy apps - if skipped_urls != []: - permissions[app + ".legacy_skipped_urls"] = { - "users": [], - "label": "Legacy permission - skipped_urls for app :" + app, - "show_tile": False, - "auth_header": False, - "public": True, - "uris": skipped_urls - } - if unprotected_urls != []: - permissions[app + ".legacy_unprotected_urls"] = { - "users": all_permissions[app + '.main']['corresponding_users'], - "label": "Legacy permission - unprotected_urls for app :" + app, - "show_tile": False, - "auth_header": True, - "public": True, - "uris": unprotected_urls - } - if protected_urls != []: - permissions[app + ".legacy_protected_urls"] = { - "users": all_permissions[app + '.main']['corresponding_users'], - "label": "Legacy permission - protected_urls for app :" + app, - "show_tile": False, - "auth_header": True, - "public": False, - "uris": protected_urls - } - - ## END Legacy part ## - # Redirected redirected_urls.update(app_settings.get('redirected_urls', {})) redirected_regex.update(app_settings.get('redirected_regex', {})) diff --git a/src/yunohost/backup.py b/src/yunohost/backup.py index 3530718d2..5018d627e 100644 --- a/src/yunohost/backup.py +++ b/src/yunohost/backup.py @@ -1292,6 +1292,7 @@ class RestoreManager(): restore_app_failed -- Raised if the restore bash script failed """ from yunohost.user import user_group_list + from yunohost.app import app_setting from yunohost.permission import permission_create, permission_delete, user_permission_list, permission_sync_to_user def copytree(src, dst, symlinks=False, ignore=None): @@ -1388,6 +1389,14 @@ class RestoreManager(): setup_group_permission = _get_migration_by_name("setup_group_permission") setup_group_permission.migrate_app_permission(app=app_instance_name) + # Migrate old settings + if app_setting(app, 'skipped_uris') is not None or \ + app_setting(app, 'unprotected_uris') is not None or \ + app_setting(app, 'protected_uris') is not None: + from yunohost.tools import _get_migration_by_name + extends_permissions_features_1 = _get_migration_by_name("extends_permissions_features_1") + extends_permissions_features_1.migrate_skipped_unprotected_protected_uris(app=app_instance_name) + # Prepare env. var. to pass to script env_dict = self._get_env_var(app_instance_name) diff --git a/src/yunohost/data_migrations/0015_extends_permissions_features_1.py b/src/yunohost/data_migrations/0015_extends_permissions_features_1.py index 9bbe8baeb..69511761d 100644 --- a/src/yunohost/data_migrations/0015_extends_permissions_features_1.py +++ b/src/yunohost/data_migrations/0015_extends_permissions_features_1.py @@ -76,6 +76,50 @@ class MyMigration(Migration): }) + def migrate_skipped_unprotected_protected_uris(self, app=None): + logger.info(m18n.n("migration_0015_migrate_old_app_settings")) + apps = _installed_apps() + + if app: + if app not in apps: + logger.error("Can't migrate permission for app %s because it ain't installed..." % app) + apps = [] + else: + apps = [app] + + def _get_setting(app, name): + s = app_setting(app, name) + return s.split(',') if s else [] + + for app in apps: + skipped_urls = [_sanitized_absolute_url(uri) for uri in app_setting(app, 'skipped_uris')] + skipped_urls += ['re:' + regex for regex in app_setting(app, 'skipped_regex')] + unprotected_urls = [_sanitized_absolute_url(uri) for uri in app_setting(app, 'unprotected_uris')] + unprotected_urls += ['re:' + regex for regex in app_setting(app, 'unprotected_regex')] + protected_urls = [_sanitized_absolute_url(uri) for uri in app_setting(app, 'protected_uris')] + protected_urls += ['re:' + regex for regex in app_setting(app, 'protected_regex')] + + if skipped_urls != []: + permission_create(app+".legacy_skipped_uris", additional_urls=skipped_urls, + auth_header=False, label='Legacy permission - skipped_urls for app : ' + app, + show_tile=False, allowed='visitors', protected=True, sync_perm=False) + if unprotected_urls != []: + permission_create(app+".legacy_unprotected_uris", additional_urls=unprotected_urls, + auth_header=True, label='Legacy permission - unprotected_uris for app : ' + app, + show_tile=False, allowed='visitors', protected=True, sync_perm=False) + if protected_urls != []: + permission_create(app+".legacy_protected_uris", additional_urls=protected_urls, + auth_header=True, label='Legacy permission - protected_uris for app : ' + app, + show_tile=False, allowed=permission_list()['permissions']['allowed'], + protected=True, sync_perm=False) + + app_setting(app, 'skipped_uris', delete=True) + app_setting(app, 'unprotected_uris', delete=True) + app_setting(app, 'protected_uris', delete=True) + + permission_sync_to_user() + + def run(self): # FIXME : what do we really want to do here ... @@ -100,7 +144,8 @@ class MyMigration(Migration): # Update LDAP database self.add_new_ldap_attributes() - app_ssowatconf() + # Migrate old settings + self.migrate_skipped_unprotected_protected_uris() except Exception as e: logger.warn(m18n.n("migration_0011_migration_failed_trying_to_rollback")) From d5c61f2d27253bf79e69c39a9034c56d61a50ce3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josu=C3=A9=20Tille?= Date: Thu, 2 Apr 2020 17:11:01 +0200 Subject: [PATCH 02/12] Manage skipped, unprotected uris in root path case --- locales/en.json | 1 + .../0015_extends_permissions_features_1.py | 12 ++++++------ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/locales/en.json b/locales/en.json index 40a5eb52b..6df38e3e5 100644 --- a/locales/en.json +++ b/locales/en.json @@ -464,6 +464,7 @@ "migration_0011_update_LDAP_schema": "Updating LDAP schema…", "migration_0011_failed_to_remove_stale_object": "Could not remove stale object {dn}: {error}", "migration_0015_add_new_attributes_in_ldap": "Add new attributes for permissions in LDAP database", + "migration_0015_migrate_old_app_settings": "Migrate old apps settings 'skipped_uris', 'unprotected_uris', 'protected_uris' in permissions system.", "migrations_already_ran": "Those migrations are already done: {ids}", "migrations_cant_reach_migration_file": "Could not access migrations files at the path '%s'", "migrations_dependencies_not_satisfied": "Run these migrations: '{dependencies_id}', before migration {id}.", diff --git a/src/yunohost/data_migrations/0015_extends_permissions_features_1.py b/src/yunohost/data_migrations/0015_extends_permissions_features_1.py index 69511761d..18077e1bf 100644 --- a/src/yunohost/data_migrations/0015_extends_permissions_features_1.py +++ b/src/yunohost/data_migrations/0015_extends_permissions_features_1.py @@ -92,12 +92,12 @@ class MyMigration(Migration): return s.split(',') if s else [] for app in apps: - skipped_urls = [_sanitized_absolute_url(uri) for uri in app_setting(app, 'skipped_uris')] - skipped_urls += ['re:' + regex for regex in app_setting(app, 'skipped_regex')] - unprotected_urls = [_sanitized_absolute_url(uri) for uri in app_setting(app, 'unprotected_uris')] - unprotected_urls += ['re:' + regex for regex in app_setting(app, 'unprotected_regex')] - protected_urls = [_sanitized_absolute_url(uri) for uri in app_setting(app, 'protected_uris')] - protected_urls += ['re:' + regex for regex in app_setting(app, 'protected_regex')] + skipped_urls = [_sanitized_absolute_url(uri) for uri in _get_setting(app, 'skipped_uris') if uri != '/'] + skipped_urls += ['re:' + regex for regex in _get_setting(app, 'skipped_regex')] + unprotected_urls = [_sanitized_absolute_url(uri) for uri in _get_setting(app, 'unprotected_uris') if uri != '/'] + unprotected_urls += ['re:' + regex for regex in _get_setting(app, 'unprotected_regex')] + protected_urls = [_sanitized_absolute_url(uri) for uri in _get_setting(app, 'protected_uris') if uri != '/'] + protected_urls += ['re:' + regex for regex in _get_setting(app, 'protected_regex')] if skipped_urls != []: permission_create(app+".legacy_skipped_uris", additional_urls=skipped_urls, From 79fb034321242c4aa4cbe38d6d4af9cad6179390 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josu=C3=A9=20Tille?= Date: Fri, 3 Apr 2020 21:27:28 +0200 Subject: [PATCH 03/12] Improve support of settings migrations --- data/helpers.d/setting | 52 ++++++++--------------- src/yunohost/app.py | 95 ++++++++++++++++++++++++++++++++++++------ 2 files changed, 100 insertions(+), 47 deletions(-) diff --git a/data/helpers.d/setting b/data/helpers.d/setting index 7edeed588..94084e534 100644 --- a/data/helpers.d/setting +++ b/data/helpers.d/setting @@ -1,7 +1,5 @@ #!/bin/bash -migrate_to_permission_deprecitated_warning="/!\\ Packagers! This app is still using the skipped/protected/unprotected_uris/regex settings which are now obsolete and deprecated... Instead, you should use the new helpers 'ynh_permission_{create,urls,update,delete}' and the 'visitors' group to initialize the public/private access. Check out the documentation at the bottom of yunohost.org/groups_and_permissions to learn how to use the new permission mechanism.\n" - # Get an application setting # # usage: ynh_app_setting_get --app=app --key=key @@ -18,7 +16,11 @@ ynh_app_setting_get() { # Manage arguments with getopts ynh_handle_getopts_args "$@" - ynh_app_setting "get" "$app" "$key" + if [[ $key =~ '^(unprotected|protected|skipped)_' ]]; then + yunohost app setting $app $key + else + ynh_app_setting "get" "$app" "$key" + fi } # Set an application setting @@ -39,7 +41,12 @@ ynh_app_setting_set() { # Manage arguments with getopts ynh_handle_getopts_args "$@" - ynh_app_setting "set" "$app" "$key" "$value" + # Manage old legacy unprotected,protectedskipped + if [[ $key =~ '^(unprotected|protected|skipped)_' ]]; then + yunohost app setting $app $key $value + else + ynh_app_setting "set" "$app" "$key" "$value" + fi } # Delete an application setting @@ -58,7 +65,13 @@ ynh_app_setting_delete() { # Manage arguments with getopts ynh_handle_getopts_args "$@" - ynh_app_setting "delete" "$app" "$key" + # Fucking legacy permission management. + # We need this because app temporarily set the app as unprotected to configure it with curl... + if [[ "$3" =~ ^(unprotected|skipped|protected)_ ]]; then + yunohost app setting $app $key -d + else + ynh_app_setting "delete" "$app" "$key" + fi } # Small "hard-coded" interface to avoid calling "yunohost app" directly each @@ -68,11 +81,6 @@ ynh_app_setting_delete() { # ynh_app_setting() { - if [[ "$1" == "delete" ]] && [[ "$3" =~ ^(unprotected|skipped)_ ]] - then - current_value=$(ynh_app_setting_get --app=$app --key=$3) - fi - ACTION="$1" APP="$2" KEY="$3" VALUE="${4:-}" python2.7 - < Date: Fri, 3 Apr 2020 22:16:28 +0200 Subject: [PATCH 04/12] Fix app_ssowatconf --- src/yunohost/app.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/yunohost/app.py b/src/yunohost/app.py index b3971a989..0ec6dedae 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -1306,10 +1306,6 @@ def app_ssowatconf(): redirected_regex = {main_domain + '/yunohost[\/]?$': 'https://' + main_domain + '/yunohost/sso/'} redirected_urls = {} - def _get_setting(settings, name): - s = settings.get(name, None) - return s.split(',') if s else [] - for app in _installed_apps(): app_settings = read_yaml(APPS_SETTING_PATH + app + '/settings.yml') From 60af7e0fe999f9948612f74bb491c08c533ecccf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josu=C3=A9=20Tille?= Date: Fri, 3 Apr 2020 22:17:50 +0200 Subject: [PATCH 05/12] Fix typo --- src/yunohost/app.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/yunohost/app.py b/src/yunohost/app.py index 0ec6dedae..0b8c679c0 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -1139,7 +1139,7 @@ def app_setting(app, key, value=None, delete=False): if value is None and not delete: try: if any(key.startswith(word+"_") for word in ["unprotected", "protected", "skipped"]): - logger.warning(legacy_settings_warning + logger.warning(legacy_settings_warning) # Well, here there are no solution to manage the root case # so just ignore this case, I don't think that get this setting # The only time that I see this is when we try to migrate to group-permission @@ -1159,7 +1159,7 @@ def app_setting(app, key, value=None, delete=False): if delete: if key in app_settings: if any(key.startswith(word+"_") for word in ["unprotected", "protected", "skipped"]): - logger.warning(legacy_settings_warning + logger.warning(legacy_settings_warning) from permission import user_permission_list, user_permission_update, permission_delete permissions = user_permission_list(full=True, full_path=False)['permissions'] From e27035f8165451b77159ba4c7261ad21744768e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josu=C3=A9=20Tille?= Date: Thu, 16 Apr 2020 16:56:57 +0200 Subject: [PATCH 06/12] Fix restore --- src/yunohost/backup.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/yunohost/backup.py b/src/yunohost/backup.py index 5018d627e..9069f265a 100644 --- a/src/yunohost/backup.py +++ b/src/yunohost/backup.py @@ -1390,9 +1390,9 @@ class RestoreManager(): setup_group_permission.migrate_app_permission(app=app_instance_name) # Migrate old settings - if app_setting(app, 'skipped_uris') is not None or \ - app_setting(app, 'unprotected_uris') is not None or \ - app_setting(app, 'protected_uris') is not None: + if app_setting(app_instance_name, 'skipped_uris') is not None or \ + app_setting(app_instance_name, 'unprotected_uris') is not None or \ + app_setting(app_instance_name, 'protected_uris') is not None: from yunohost.tools import _get_migration_by_name extends_permissions_features_1 = _get_migration_by_name("extends_permissions_features_1") extends_permissions_features_1.migrate_skipped_unprotected_protected_uris(app=app_instance_name) From c4f7fc2baca452bd8561bfda072c845daa352c26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josu=C3=A9=20Tille?= Date: Tue, 21 Apr 2020 11:24:25 +0200 Subject: [PATCH 07/12] Remove migration of legacy settings in install --- src/yunohost/app.py | 30 ------------------------------ 1 file changed, 30 deletions(-) diff --git a/src/yunohost/app.py b/src/yunohost/app.py index 0b8c679c0..73a2a34d2 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -866,8 +866,6 @@ def app_install(operation_logger, app, label=None, args=None, no_remove_on_failu permission_url(app_instance_name + ".main", url='/', sync_perm=False) user_permission_update(app_instance_name + ".main", show_tile=True, sync_perm=False) - _migrate_legacy_permissions(app_instance_name) - permission_sync_to_user() logger.success(m18n.n('installation_complete')) @@ -902,34 +900,6 @@ def dump_app_log_extract_for_debugging(operation_logger): logger.info(line) -def _migrate_legacy_permissions(app): - - from yunohost.permission import user_permission_list, user_permission_update - - # Check if app is apparently using the legacy permission management, defined by the presence of something like - # ynh_app_setting_set on unprotected_uris (or yunohost app setting) - install_script_path = os.path.join(APPS_SETTING_PATH, app, 'scripts/install') - install_script_content = open(install_script_path, "r").read() - if not re.search(r"(yunohost app setting|ynh_app_setting_set) .*(unprotected|skipped)_uris", install_script_content): - return - - app_settings = _get_app_settings(app) - app_perm_currently_allowed = user_permission_list()["permissions"][app + ".main"]["allowed"] - - settings_say_it_should_be_public = (app_settings.get("unprotected_uris", None) == "/" - or app_settings.get("skipped_uris", None) == "/") - - # If the current permission says app is protected, but there are legacy rules saying it should be public... - if app_perm_currently_allowed == ["all_users"] and settings_say_it_should_be_public: - # Make it public - user_permission_update(app + ".main", add="visitors", sync_perm=False) - - # If the current permission says app is public, but there are no setting saying it should be public... - if app_perm_currently_allowed == ["visitors"] and not settings_say_it_should_be_public: - # Make is private - user_permission_update(app + ".main", remove="visitors", sync_perm=False) - - @is_unit_operation() def app_remove(operation_logger, app): """ From e068133b54162703ea3ea9d27a76107cbc9da1f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josu=C3=A9=20Tille?= Date: Tue, 21 Apr 2020 17:14:52 +0200 Subject: [PATCH 08/12] Fix apps settings management --- data/helpers.d/setting | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/data/helpers.d/setting b/data/helpers.d/setting index 94084e534..5152e844a 100644 --- a/data/helpers.d/setting +++ b/data/helpers.d/setting @@ -16,7 +16,7 @@ ynh_app_setting_get() { # Manage arguments with getopts ynh_handle_getopts_args "$@" - if [[ $key =~ '^(unprotected|protected|skipped)_' ]]; then + if [[ $key =~ (unprotected|protected|skipped)_ ]]; then yunohost app setting $app $key else ynh_app_setting "get" "$app" "$key" @@ -42,8 +42,8 @@ ynh_app_setting_set() { ynh_handle_getopts_args "$@" # Manage old legacy unprotected,protectedskipped - if [[ $key =~ '^(unprotected|protected|skipped)_' ]]; then - yunohost app setting $app $key $value + if [[ $key =~ (unprotected|protected|skipped)_ ]]; then + yunohost app setting $app $key -v $value else ynh_app_setting "set" "$app" "$key" "$value" fi @@ -67,7 +67,7 @@ ynh_app_setting_delete() { # Fucking legacy permission management. # We need this because app temporarily set the app as unprotected to configure it with curl... - if [[ "$3" =~ ^(unprotected|skipped|protected)_ ]]; then + if [[ "$key" =~ (unprotected|skipped|protected)_ ]]; then yunohost app setting $app $key -d else ynh_app_setting "delete" "$app" "$key" From 76c7bcc69b2ab90fcc927f8b91f1e87d248bbd06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josu=C3=A9=20Tille?= Date: Wed, 22 Apr 2020 14:46:26 +0200 Subject: [PATCH 09/12] Fix some typos --- src/yunohost/app.py | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/yunohost/app.py b/src/yunohost/app.py index 73a2a34d2..914220a81 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -1155,16 +1155,14 @@ def app_setting(app, key, value=None, delete=False): if urls == '/': if key.startswith("unprotected_") or key.startswith("skipped_"): + permission_url(app + ".main", url='/', sync_perm=False) user_permission_update(app + ".main", add="visitors") else: user_permission_update(app + ".main", remove="visitors") else: - # Add re: in case of regex, as we distingish regex by this since the permission + # Add re: in case of regex, as we distingish regex by this since the new permission system if key.endswith('_regex'): - if urls.startswith('/'): - urls = 're:' + urls - else: - urls = 're:/' + urls + urls = 're:' + urls permissions = user_permission_list(full=True, full_path=False)['permissions'] if permission_name in permissions: @@ -1179,15 +1177,15 @@ def app_setting(app, key, value=None, delete=False): new_urls = urls.split(',') + actuals_urls_or_regex # We need to clear urls because in the old setting the new setting override the old one and dont just add some urls - permission_url(clear_url=True, sync_perm=False) - permission_url(add_url=new_urls) + permission_url(permission_name, clear_urls=True, sync_perm=False) + permission_url(permission_name, add_url=new_urls) else: # Let's create a "special" permission for the legacy settings - permission_create(permission=permission, + permission_create(permission=permission_name, # FIXME find a way to limit to only the user allowed to the main permission allowed=['all_users'] if key.startswith('protected_') else ['all_users', 'visitors'], url=None, - additional_urls=url.split(','), + additional_urls=urls.split(','), auth_header=not key.startswith('skipped_'), label="Legacy permission - %s_uris/regex for app : %s" % (key.split('_')[0], app), show_tile=False, From 815c8fc49e3b499fba9691f44dfc6d531ef56542 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Thu, 24 Sep 2020 20:25:28 +0200 Subject: [PATCH 10/12] Refactor app_setting to better separate legacy permission setting vs. regular setting, hopefully make the core more readable --- data/helpers.d/setting | 3 -- src/yunohost/app.py | 114 ++++++++++++++++++++++------------------- 2 files changed, 62 insertions(+), 55 deletions(-) diff --git a/data/helpers.d/setting b/data/helpers.d/setting index 99fd8b244..e122e2e82 100644 --- a/data/helpers.d/setting +++ b/data/helpers.d/setting @@ -41,7 +41,6 @@ ynh_app_setting_set() { # Manage arguments with getopts ynh_handle_getopts_args "$@" - # Manage old legacy unprotected,protectedskipped if [[ $key =~ (unprotected|protected|skipped)_ ]]; then yunohost app setting $app $key -v $value else @@ -65,8 +64,6 @@ ynh_app_setting_delete() { # Manage arguments with getopts ynh_handle_getopts_args "$@" - # Fucking legacy permission management. - # We need this because app temporarily set the app as unprotected to configure it with curl... if [[ "$key" =~ (unprotected|skipped|protected)_ ]]; then yunohost app setting $app $key -d else diff --git a/src/yunohost/app.py b/src/yunohost/app.py index 15f0156cc..13ff1117a 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -1144,55 +1144,51 @@ def app_setting(app, key, value=None, delete=False): """ app_settings = _get_app_settings(app) or {} - legacy_settings_warning="/!\\ Packagers! This app is still using the skipped/protected/unprotected_uris/regex settings which are now obsolete and deprecated... Instead, you should use the new helpers 'ynh_permission_{create,urls,update,delete}' and the 'visitors' group to initialize the public/private access. Check out the documentation at the bottom of yunohost.org/groups_and_permissions to learn how to use the new permission mechanism." - if value is None and not delete: - try: - if any(key.startswith(word+"_") for word in ["unprotected", "protected", "skipped"]): - logger.warning(legacy_settings_warning) - # Well, here there are no solution to manage the root case - # so just ignore this case, I don't think that get this setting - # The only time that I see this is when we try to migrate to group-permission - from permission import user_permission_list - permissions = user_permission_list(full=True, full_path=False)['permissions'] - permission_name = "%s.legacy_%s_uris" % (app, key.split('_')[0]) - if permission_name in permissions: - return ','.join(permissions[permission_name]['additional_urls']) - else: - return None - else: - return app_settings[key] - except Exception as e: - logger.debug("cannot get app setting '%s' for '%s' (%s)", key, app, e) - return None + # + # Legacy permission setting management + # (unprotected, protected, skipped_uri/regex) + # - if delete: - if key in app_settings: - if any(key.startswith(word+"_") for word in ["unprotected", "protected", "skipped"]): - logger.warning(legacy_settings_warning) - from permission import user_permission_list, user_permission_update, permission_delete + is_legacy_permission_setting = any(key.startswith(word+"_") for word in ["unprotected", "protected", "skipped"]) - permissions = user_permission_list(full=True, full_path=False)['permissions'] + if is_legacy_permission_setting: - # In in case of the visitors group is in the main permission, it's probably that - # we wan't to remove this group so just to dit - if 'visitors' in permissions[app + ".main"]['allowed'] and 'is_public' in app_settings: + logger.warning("/!\\ Packagers! This app is still using the skipped/protected/unprotected_uris/regex settings which are now obsolete and deprecated... Instead, you should use the new helpers 'ynh_permission_{create,urls,update,delete}' and the 'visitors' group to initialize the public/private access. Check out the documentation at the bottom of yunohost.org/groups_and_permissions to learn how to use the new permission mechanism.") + from permission import user_permission_list, user_permission_update, permission_create, permission_delete, permission_url + permissions = user_permission_list(full=True, full_path=False)['permissions'] + permission_name = "%s.legacy_%s_uris" % (app, key.split('_')[0]) + permission = permission.get(permission_name) + + # GET + if value is None and not delete: + # FIXME FIXME FIXME : what about the main url ...? + return ','.join(permission['additional_urls']) if permission else None + + # DELETE + if delete: + if key in app_settings: + + # If 'is_public' setting still exists, we interpret this as + # coming from a legacy app (because new apps should manage the + # is_public state themselves anymore...) + # + # In that case, we interpret the request for "deleting + # unprotected/skipped" setting as willing to make the app + # private + if 'is_public' in app_settings and 'visitors' in permissions[app + ".main"]['allowed']: if key.startswith('unprotected_') or key.startswith('skipped_'): user_permission_update(app + ".main", remove="visitors") else: - permission_name = "%s.legacy_%s_uris" % (app, key.split('_')[0]) - if permission_name in permissions: + if permission: permission_delete(permission_name) - else: - del app_settings[key] - else: - if any(key.startswith(word+"_") for word in ["unprotected", "protected", "skipped"]): - logger.warning(legacy_settings_warning) - from permission import user_permission_list, user_permission_update, permission_create, permission_url + # SET + else: urls = value - permission_name = "%s.legacy_%s_uris" % (app, key.split('_')[0]) - + # If the request is about the root of the app (/), ( = the vast majority of cases) + # we interpret this as a change for the main permission + # (i.e. allowing/disallowing visitors) if urls == '/': if key.startswith("unprotected_") or key.startswith("skipped_"): permission_url(app + ".main", url='/', sync_perm=False) @@ -1204,18 +1200,17 @@ def app_setting(app, key, value=None, delete=False): if key.endswith('_regex'): urls = 're:' + urls - permissions = user_permission_list(full=True, full_path=False)['permissions'] - if permission_name in permissions: + if permission: # In case of new regex, save the urls, to add a new time in the additional_urls # In case of new urls, we do the same thing but inversed if key.endswith('_regex'): # List of urls to save - actuals_urls_or_regex = [url for url in permissions[permission_name]['additional_urls'] if not url.startswith('re:')] + current_urls_or_regex = [url for url in permission['additional_urls'] if not url.startswith('re:')] else: # List of regex to save - actuals_urls_or_regex = [url for url in permissions[permission_name]['additional_urls'] if url.startswith('re:')] + current_urls_or_regex = [url for url in permission['additional_urls'] if url.startswith('re:')] - new_urls = urls.split(',') + actuals_urls_or_regex + new_urls = urls.split(',') + current_urls_or_regex # We need to clear urls because in the old setting the new setting override the old one and dont just add some urls permission_url(permission_name, clear_urls=True, sync_perm=False) permission_url(permission_name, add_url=new_urls) @@ -1223,20 +1218,35 @@ def app_setting(app, key, value=None, delete=False): # Let's create a "special" permission for the legacy settings permission_create(permission=permission_name, # FIXME find a way to limit to only the user allowed to the main permission - allowed=['all_users'] if key.startswith('protected_') else ['all_users', 'visitors'], + allowed=['all_users'] if key.startswith('protected_') else ['all_users', 'visitors'], url=None, additional_urls=urls.split(','), auth_header=not key.startswith('skipped_'), label="Legacy permission - %s_uris/regex for app : %s" % (key.split('_')[0], app), - show_tile=False, + show_tile=False, protected=True) - else: - # FIXME: Allow multiple values for some keys? - if key in ['redirected_urls', 'redirected_regex']: - value = yaml.load(value) - app_settings[key] = value - _set_app_settings(app, app_settings) + + # + # Regular setting management + # + + # GET + if value is None and not delete: + return app_settings.get(key, None) + + # DELETE + if delete: + if key in app_settings: + del app_settings[key] + + # SET + else: + if key in ['redirected_urls', 'redirected_regex']: + value = yaml.load(value) + app_settings[key] = value + + _set_app_settings(app, app_settings) def app_register_url(app, domain, path): From 2b4e14cca7b27ce21a88d18de290671015149f67 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Thu, 24 Sep 2020 21:46:33 +0200 Subject: [PATCH 11/12] Hardcode some permission labels for non-trivial legacy permissions --- src/yunohost/app.py | 5 +-- .../0019_extends_permissions_features_1.py | 7 ++-- src/yunohost/utils/legacy.py | 32 +++++++++++++++++++ 3 files changed, 39 insertions(+), 5 deletions(-) diff --git a/src/yunohost/app.py b/src/yunohost/app.py index 13ff1117a..ab79b79fa 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -1215,14 +1215,15 @@ def app_setting(app, key, value=None, delete=False): permission_url(permission_name, clear_urls=True, sync_perm=False) permission_url(permission_name, add_url=new_urls) else: - # Let's create a "special" permission for the legacy settings + from utils.legacy import legacy_permission_label + # Let's create a "special" permission for the legacy settings permission_create(permission=permission_name, # FIXME find a way to limit to only the user allowed to the main permission allowed=['all_users'] if key.startswith('protected_') else ['all_users', 'visitors'], url=None, additional_urls=urls.split(','), auth_header=not key.startswith('skipped_'), - label="Legacy permission - %s_uris/regex for app : %s" % (key.split('_')[0], app), + label=legacy_permission_label(app, key.split('_')[0]), show_tile=False, protected=True) diff --git a/src/yunohost/data_migrations/0019_extends_permissions_features_1.py b/src/yunohost/data_migrations/0019_extends_permissions_features_1.py index 63732800a..81aebba73 100644 --- a/src/yunohost/data_migrations/0019_extends_permissions_features_1.py +++ b/src/yunohost/data_migrations/0019_extends_permissions_features_1.py @@ -77,6 +77,7 @@ class MyMigration(Migration): def migrate_skipped_unprotected_protected_uris(self, app=None): + from utils.legacy import legacy_permission_label logger.info(m18n.n("migration_0019_migrate_old_app_settings")) apps = _installed_apps() @@ -101,15 +102,15 @@ class MyMigration(Migration): if skipped_urls != []: permission_create(app+".legacy_skipped_uris", additional_urls=skipped_urls, - auth_header=False, label='Legacy permission - skipped_urls for app : ' + app, + auth_header=False, label=legacy_permission_label(app, "skipped"), show_tile=False, allowed='visitors', protected=True, sync_perm=False) if unprotected_urls != []: permission_create(app+".legacy_unprotected_uris", additional_urls=unprotected_urls, - auth_header=True, label='Legacy permission - unprotected_uris for app : ' + app, + auth_header=True, label=legacy_permission_label(app, "unprotected"), show_tile=False, allowed='visitors', protected=True, sync_perm=False) if protected_urls != []: permission_create(app+".legacy_protected_uris", additional_urls=protected_urls, - auth_header=True, label='Legacy permission - protected_uris for app : ' + app, + auth_header=True, label=legacy_permission_label(app, "protected"), show_tile=False, allowed=permission_list()['permissions']['allowed'], protected=True, sync_perm=False) diff --git a/src/yunohost/utils/legacy.py b/src/yunohost/utils/legacy.py index e7f584d6a..1045bf9f2 100644 --- a/src/yunohost/utils/legacy.py +++ b/src/yunohost/utils/legacy.py @@ -112,3 +112,35 @@ class SetupGroupPermissions(): user_permission_update(app + ".main", add="visitors", sync_perm=False) permission_sync_to_user() + +LEGACY_PERMISSION_LABEL = { + ("nextcloud": "skipped"): "api ", # .well-known + ("libreto": "skipped"): "pad access", # /[^/]+ + ("leed": "skipped"): "api", # /action.php, for cron task ... + ("mailman": "protected"): "admin", # /admin + ("prettynoemiecms": "protected"): "admin", # /admin + ("etherpad_mypads": "skipped"): "admin", # /admin + ("baikal": "protected"): "admin", # /admin/ + ("couchpotato": "unprotected"): "api", # /api + ("freshrss": "skipped"): "api", # /api/, + ("portainer": "skipped"): "api", # /api/webhooks/ + ("jeedom": "unprotected"): "api", # /core/api/jeeApi.php + ("bozon": "protected"): "user interface", # /index.php + ("limesurvey": "protected"): "admin ", # /index.php?r=admin,/index.php?r=plugins,/scripts + ("kanboard": "unprotected"): "api ", # /jsonrpc.php + ("seafile": "unprotected"): "medias", # /media + ("ttrss": "skipped"): "api", # /public.php,/api,/opml.php?op=publish + ("libreerp": "protected"): "admin ", # /web/database/manager + ("z-push": "skipped"): "api ", # $domain/[Aa]uto[Dd]iscover/.* + ("radicale": "skipped"): "?", # $domain$path_url + ("jirafeau": "protected"): "user interface", # $domain$path_url/$","$domain$path_url/admin.php.*$ + ("opensondage": "protected"): "admin", # $domain$path_url/admin/ + ("lstu": "protected"): "user interface", # $domain$path_url/login$","$domain$path_url/logout$","$domain$path_url/api$","$domain$path_url/extensions$","$domain$path_url/stats$","$domain$path_url/d/.*$","$domain$path_url/a$","$domain$path_url/$ + ("lutim": "protected"): "user interface", # $domain$path_url/stats/?$","$domain$path_url/manifest.webapp/?$","$domain$path_url/?$","$domain$path_url/[d-m]/.*$ + ("lufi": "protected"): "user interface", # $domain$path_url/stats$","$domain$path_url/manifest.webapp$","$domain$path_url/$","$domain$path_url/d/.*$","$domain$path_url/m/.*$ + ("gogs": "skipped"): "api ", # $excaped_domain$excaped_path/[%w-.]*/[%w-.]*/git%-receive%-pack,$excaped_domain$excaped_path/[%w-.]*/[%w-.]*/git%-upload%-pack,$excaped_domain$excaped_path/[%w-.]*/[%w-.]*/info/refs + + } + +def legacy_permission_label(app, permission_type): + return LEGACY_PERMISSION_LABEL.get((app, permission_type), "Legacy %s urls" % permission_type) From b55d8e023f677612a0ed9b0686fd28bf0efa15f8 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Thu, 24 Sep 2020 21:55:34 +0200 Subject: [PATCH 12/12] Typo --- 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 ab79b79fa..494c86df5 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -1158,7 +1158,7 @@ def app_setting(app, key, value=None, delete=False): from permission import user_permission_list, user_permission_update, permission_create, permission_delete, permission_url permissions = user_permission_list(full=True, full_path=False)['permissions'] permission_name = "%s.legacy_%s_uris" % (app, key.split('_')[0]) - permission = permission.get(permission_name) + permission = permissions.get(permission_name) # GET if value is None and not delete: