From e7d1cc5f9449f77bf575c82f9faae5f2f2168b41 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Wed, 9 Oct 2019 22:55:06 +0200 Subject: [PATCH] Allow to specify right away what groups to allow for a permission when creating it --- data/helpers.d/setting | 16 +++++++++++----- src/yunohost/app.py | 2 +- src/yunohost/backup.py | 11 ++++------- .../0011_setup_group_permission.py | 8 +++++--- src/yunohost/permission.py | 16 ++++++++++++++-- src/yunohost/tests/test_permission.py | 9 +++++++++ 6 files changed, 44 insertions(+), 18 deletions(-) diff --git a/data/helpers.d/setting b/data/helpers.d/setting index c911c5811..a8d2919a4 100644 --- a/data/helpers.d/setting +++ b/data/helpers.d/setting @@ -232,11 +232,12 @@ ynh_webpath_register () { # Create a new permission for the app # -# example: ynh_permission_create --permission admin --url /admin +# example: ynh_permission_create --permission admin --url /admin --allowed alice bob # -# usage: ynh_permission_create --permission "permission" [--url "url"] +# usage: ynh_permission_create --permission "permission" [--url "url"] [--allowed group1 group2] # | arg: permission - the name for the permission (by default a permission named "main" already exist) # | arg: url - (optional) URL for which access will be allowed/forbidden +# | arg: allowed - (optional) A list of group/user to allow for the permission # # If provided, 'url' is assumed to be relative to the app domain/path if they # start with '/'. For example: @@ -250,9 +251,10 @@ ynh_webpath_register () { # re:domain.tld/app/api/[A-Z]*$ -> domain.tld/app/api/[A-Z]*$ # ynh_permission_create() { - declare -Ar args_array=( [p]=permission= [u]=url= ) + declare -Ar args_array=( [p]=permission= [u]=url= [a]=allowed= ) local permission - local urls + local url + local allowed ynh_handle_getopts_args "$@" if [[ -n ${url:-} ]]; then @@ -261,7 +263,11 @@ ynh_permission_create() { url="None" fi - yunohost tools shell -c "from yunohost.permission import permission_create; permission_create('$app.$permission', url=$url, sync_perm=False)" + if [[ -n ${allowed:-} ]]; then + allowed=",allowed=['${allowed//';'/"','"}']" + fi + + yunohost tools shell -c "from yunohost.permission import permission_create; permission_create('$app.$permission', url=$url ${allowed:-} , sync_perm=False)" } # Remove a permission for the app (note that when the app is removed all permission is automatically removed) diff --git a/src/yunohost/app.py b/src/yunohost/app.py index 75bf12f3d..7235535cd 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -981,7 +981,7 @@ def app_install(operation_logger, app, label=None, args=None, no_remove_on_failu # Initialize the main permission for the app # After the install, if apps don't have a domain and path defined, the default url '/' is removed from the permission - permission_create(app_instance_name+".main", url="/") + permission_create(app_instance_name+".main", url="/", allowed=["all_users"]) # Execute the app install script install_retcode = 1 diff --git a/src/yunohost/backup.py b/src/yunohost/backup.py index 90f795ea5..c57ab6685 100644 --- a/src/yunohost/backup.py +++ b/src/yunohost/backup.py @@ -1251,8 +1251,7 @@ class RestoreManager(): for permission_name, permission_infos in old_apps_permission.items(): app_name = permission_name.split(".")[0] if _is_installed(app_name): - permission_create(permission_name, url=permission_infos["url"], sync_perm=False) - user_permission_update(permission_name, remove="all_users", add=permission_infos["allowed"], sync_perm=False) + permission_create(permission_name, url=permission_infos["url"], allowed=permission_infos["allowed"], sync_perm=False) permission_sync_to_user() @@ -1365,15 +1364,13 @@ class RestoreManager(): for permission_name, permission_infos in permissions.items(): - permission_create(permission_name, url=permission_infos.get("url", None), sync_perm=False) - if "allowed" not in permission_infos: logger.warning("'allowed' key corresponding to allowed groups for permission %s not found when restoring app %s … You might have to reconfigure permissions yourself." % (permission_name, app_instance_name)) + should_be_allowed = ["all_users"] else: should_be_allowed = [g for g in permission_infos["allowed"] if g in existing_groups] - current_allowed = user_permission_list()["permissions"][permission_name]["allowed"] - if should_be_allowed != current_allowed: - user_permission_update(permission_name, remove=current_allowed, add=should_be_allowed, sync_perm=False) + + permission_create(permission_name, url=permission_infos.get("url", None), allowed=should_be_allowed, sync_perm=False) permission_sync_to_user() diff --git a/src/yunohost/data_migrations/0011_setup_group_permission.py b/src/yunohost/data_migrations/0011_setup_group_permission.py index 880c5f54b..3114817b9 100644 --- a/src/yunohost/data_migrations/0011_setup_group_permission.py +++ b/src/yunohost/data_migrations/0011_setup_group_permission.py @@ -108,10 +108,12 @@ class MyMigration(Migration): domain = app_setting(app, 'domain') url = "/" if domain and path else None - permission_create(app+".main", url=url, sync_perm=False) if permission: - allowed_group = permission.split(',') - user_permission_update(app+".main", remove="all_users", add=allowed_group, sync_perm=False) + allowed_groups = permission.split(',') + else: + allowed_groups = ["all_users"] + permission_create(app+".main", url=url, allowed=allowed_groups, sync_perm=False) + app_setting(app, 'allowed_users', delete=True) # Migrate classic public app still using the legacy unprotected_uris diff --git a/src/yunohost/permission.py b/src/yunohost/permission.py index 6f9d63d69..426ecd10f 100644 --- a/src/yunohost/permission.py +++ b/src/yunohost/permission.py @@ -266,13 +266,14 @@ def user_permission_reset(operation_logger, permission, sync_perm=True): @is_unit_operation() -def permission_create(operation_logger, permission, url=None, sync_perm=True): +def permission_create(operation_logger, permission, url=None, allowed=None, sync_perm=True): """ Create a new permission for a specific application Keyword argument: permission -- Name of the permission (e.g. mail or nextcloud or wordpress.editors) url -- (optional) URL for which access will be allowed/forbidden + allowed -- (optional) A list of group/user to allow for the permission If provided, 'url' is assumed to be relative to the app domain/path if they start with '/'. For example: @@ -286,6 +287,7 @@ def permission_create(operation_logger, permission, url=None, sync_perm=True): re:domain.tld/app/api/[A-Z]*$ -> domain.tld/app/api/[A-Z]*$ """ + from yunohost.user import user_group_list from yunohost.utils.ldap import _get_ldap_interface ldap = _get_ldap_interface() @@ -312,8 +314,18 @@ def permission_create(operation_logger, permission, url=None, sync_perm=True): 'gidNumber': gid, } + # If who should be allowed is explicitly provided, use this info + if allowed: + if not isinstance(allowed, list): + allowed = [allowed] + # (though first we validate that the targets actually exist) + all_existing_groups = user_group_list()['groups'].keys() + for g in allowed: + if g not in all_existing_groups: + raise YunohostError('group_unknown', group=g) + attr_dict['groupPermission'] = ['cn=%s,ou=groups,dc=yunohost,dc=org' % g for g in allowed] # For main permission, we add all users by default - if permission.endswith(".main"): + elif permission.endswith(".main"): attr_dict['groupPermission'] = ['cn=all_users,ou=groups,dc=yunohost,dc=org'] if url: diff --git a/src/yunohost/tests/test_permission.py b/src/yunohost/tests/test_permission.py index 8e536ec9e..5e1246793 100644 --- a/src/yunohost/tests/test_permission.py +++ b/src/yunohost/tests/test_permission.py @@ -226,6 +226,15 @@ def test_permission_create_extra(): assert "all_users" not in res['site.test']['allowed'] assert res['site.test']['corresponding_users'] == [] + +def test_permission_create_with_allowed(): + permission_create("site.test", allowed=["alice"]) + + res = user_permission_list(full=True)['permissions'] + assert "site.test" in res + assert res['site.test']['allowed'] == ["alice"] + + def test_permission_delete(): permission_delete("wiki.main", force=True)