From ebf2fb9a141da65954d1855bc025f0cf362c6f99 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Fri, 20 Sep 2019 20:13:51 +0200 Subject: [PATCH] Use relative urls by default for permissions while still supporting absolute urls ... --- src/yunohost/app.py | 18 +++++------- .../0011_setup_group_permission.py | 2 +- src/yunohost/permission.py | 29 ++++++++++--------- 3 files changed, 23 insertions(+), 26 deletions(-) diff --git a/src/yunohost/app.py b/src/yunohost/app.py index bba5fb104..231568439 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -553,8 +553,6 @@ def app_change_url(operation_logger, app, domain, path): app_setting(app, 'domain', value=domain) app_setting(app, 'path', value=path) - permission_urls(app+".main", add=[domain+path], remove=[old_domain+old_path], sync_perm=True) - # avoid common mistakes if _run_service_command("reload", "nginx") is False: # grab nginx errors @@ -868,10 +866,9 @@ def app_install(operation_logger, app, label=None, args=None, no_remove_on_failu if os.path.exists(os.path.join(extracted_app_folder, file_to_copy)): os.system('cp -R %s/%s %s' % (extracted_app_folder, file_to_copy, app_setting_path)) - # Create permission before the install (useful if the install script redefine the permission) - # Note that sync_perm is disabled to avoid triggering a whole bunch of code and messages - # can't be sure that we don't have one case when it's needed - permission_create(app_instance_name+".main", sync_perm=False) + # 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", urls=["/"]) # Execute the app install script install_retcode = 1 @@ -949,17 +946,16 @@ def app_install(operation_logger, app, label=None, args=None, no_remove_on_failu os.system('chown -R root: %s' % app_setting_path) os.system('chown -R admin: %s/scripts' % app_setting_path) - # Add path in permission if it's defined in the app install script + # If an app doesn't have at least a domain and a path, assume it's not a webapp and remove the default "/" permission app_settings = _get_app_settings(app_instance_name) domain = app_settings.get('domain', None) path = app_settings.get('path', None) - if domain and path: - # FIXME : might want to move this to before running the install script because some app need to run install script during initialization etc (idk) ? - permission_urls(app_instance_name+".main", add=[domain+path], sync_perm=False) + if not (domain and path): + permission_urls(app_instance_name + ".main", remove=["/"], sync_perm=False) # Migrate classic public app still using the legacy unprotected_uris if app_settings.get("unprotected_uris", None) == "/": - user_permission_update(app_instance_name+".main", remove="all_users", add="visitors", sync_perm=False) + user_permission_update(app_instance_name + ".main", remove="all_users", add="visitors", 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 a99dfb7c1..dd5b3c274 100644 --- a/src/yunohost/data_migrations/0011_setup_group_permission.py +++ b/src/yunohost/data_migrations/0011_setup_group_permission.py @@ -108,7 +108,7 @@ class MyMigration(Migration): path = app_setting(app, 'path') domain = app_setting(app, 'domain') - urls = [domain + path] if domain and path else None + urls = "/" if domain and path else None permission_create(app+".main", urls=urls, sync_perm=False) if permission: allowed_group = permission.split(',') diff --git a/src/yunohost/permission.py b/src/yunohost/permission.py index dbfc6e6f5..5f9a88e11 100644 --- a/src/yunohost/permission.py +++ b/src/yunohost/permission.py @@ -268,7 +268,18 @@ def permission_create(operation_logger, permission, urls=None, sync_perm=True): Keyword argument: permission -- Name of the permission (e.g. mail or nextcloud or wordpress.editors) - urls -- list of urls to specify for the permission + urls -- list of urls to specify for the permission. + + Urls are assumed to be relative to the app domain/path if they start with '/'. + For example: + / -> domain.tld/app + /admin -> domain.tld/app/admin + domain.tld/app/api -> domain.tld/app/api + + Urls can be later treated as regexes when they start with "re:". + For example: + re:/api/[A-Z]*$ -> domain.tld/app/api/[A-Z]*$ + re:domain.tld/app/api/[A-Z]*$ -> domain.tld/app/api/[A-Z]*$ """ from yunohost.utils.ldap import _get_ldap_interface @@ -302,7 +313,7 @@ def permission_create(operation_logger, permission, urls=None, sync_perm=True): attr_dict['groupPermission'] = ['cn=all_users,ou=groups,dc=yunohost,dc=org'] if urls: - attr_dict['URL'] = [_normalize_url(url) for url in urls] + attr_dict['URL'] = urls operation_logger.related_to.append(('app', permission.split(".")[0])) operation_logger.start() @@ -326,8 +337,8 @@ def permission_urls(operation_logger, permission, add=None, remove=None, sync_pe Keyword argument: permission -- Name of the permission (e.g. mail or nextcloud or wordpress.editors) - add -- List of urls to add - remove -- List of urls to remove + add -- List of urls to add (c.f. permission_create for documentation about their format) + remove -- List of urls to remove (c.f. permission_create for documentation about their format) """ from yunohost.utils.ldap import _get_ldap_interface @@ -345,11 +356,9 @@ def permission_urls(operation_logger, permission, add=None, remove=None, sync_pe if add: urls_to_add = [add] if not isinstance(add, list) else add - urls_to_add = [_normalize_url(url) for url in urls_to_add] new_urls += urls_to_add if remove: urls_to_remove = [remove] if not isinstance(remove, list) else remove - urls_to_remove = [_normalize_url(url) for url in urls_to_remove] new_urls = [u for u in new_urls if u not in urls_to_remove] if set(new_urls) == set(existing_permission["urls"]): @@ -457,11 +466,3 @@ def permission_sync_to_user(): # Reload unscd, otherwise the group ain't propagated to the LDAP database os.system('nscd --invalidate=passwd') os.system('nscd --invalidate=group') - - -def _normalize_url(url): - from yunohost.domain import _normalize_domain_path - domain = url[:url.index('/')] - path = url[url.index('/'):] - domain, path = _normalize_domain_path(domain, path) - return domain + path