diff --git a/src/yunohost/app.py b/src/yunohost/app.py index c93c2a7f9..308032a61 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -317,7 +317,7 @@ def app_change_url(operation_logger, app, domain, path): """ from yunohost.hook import hook_exec, hook_callback - from yunohost.domain import _normalize_domain_path, _get_conflicting_apps + from yunohost.domain import _normalize_domain_path, _assert_no_conflicting_apps installed = _is_installed(app) if not installed: @@ -337,17 +337,7 @@ def app_change_url(operation_logger, app, domain, path): raise YunohostError("app_change_url_identical_domains", domain=domain, path=path) # Check the url is available - conflicts = _get_conflicting_apps(domain, path, ignore_app=app) - if conflicts: - apps = [] - for path, app_id, app_label in conflicts: - apps.append(" * {domain:s}{path:s} → {app_label:s} ({app_id:s})".format( - domain=domain, - path=path, - app_id=app_id, - app_label=app_label, - )) - raise YunohostError('app_location_unavailable', apps="\n".join(apps)) + _assert_no_conflicting_apps(domain, path, ignore_app=app) manifest = _get_manifest_of_app(os.path.join(APPS_SETTING_PATH, app)) @@ -1275,7 +1265,7 @@ def app_register_url(app, domain, path): # This line can't be moved on top of file, otherwise it creates an infinite # loop of import with tools.py... - from .domain import _get_conflicting_apps, _normalize_domain_path + from .domain import _assert_no_conflicting_apps, _normalize_domain_path domain, path = _normalize_domain_path(domain, path) @@ -1288,18 +1278,7 @@ def app_register_url(app, domain, path): raise YunohostError('app_already_installed_cant_change_url') # Check the url is available - conflicts = _get_conflicting_apps(domain, path) - if conflicts: - apps = [] - for path, app_id, app_label in conflicts: - apps.append(" * {domain:s}{path:s} → {app_label:s} ({app_id:s})".format( - domain=domain, - path=path, - app_id=app_id, - app_label=app_label, - )) - - raise YunohostError('app_location_unavailable', apps="\n".join(apps)) + _assert_no_conflicting_apps(domain, path) app_setting(app, 'domain', value=domain) app_setting(app, 'path', value=path) @@ -2655,7 +2634,7 @@ def _parse_args_in_yunohost_format(user_answers, argument_questions): def _validate_and_normalize_webpath(manifest, args_dict, app_folder): - from yunohost.domain import _get_conflicting_apps, _normalize_domain_path + from yunohost.domain import _assert_no_conflicting_apps, _normalize_domain_path # If there's only one "domain" and "path", validate that domain/path # is an available url and normalize the path. @@ -2670,18 +2649,7 @@ def _validate_and_normalize_webpath(manifest, args_dict, app_folder): domain, path = _normalize_domain_path(domain, path) # Check the url is available - conflicts = _get_conflicting_apps(domain, path) - if conflicts: - apps = [] - for path, app_id, app_label in conflicts: - apps.append(" * {domain:s}{path:s} → {app_label:s} ({app_id:s})".format( - domain=domain, - path=path, - app_id=app_id, - app_label=app_label, - )) - - raise YunohostError('app_location_unavailable', apps="\n".join(apps)) + _assert_no_conflicting_apps(domain, path) # (We save this normalized path so that the install script have a # standard path format to deal with no matter what the user inputted) @@ -2704,8 +2672,7 @@ def _validate_and_normalize_webpath(manifest, args_dict, app_folder): and re.search(r"(ynh_webpath_register|yunohost app checkurl)", install_script_content): domain = domain_args[0][1] - if _get_conflicting_apps(domain, "/"): - raise YunohostError('app_full_domain_unavailable', domain=domain) + _assert_no_conflicting_apps(domain, "/") def _make_environment_dict(args_dict, prefix="APP_ARG_"): diff --git a/src/yunohost/domain.py b/src/yunohost/domain.py index 4d2b6ab53..b3ef1299e 100644 --- a/src/yunohost/domain.py +++ b/src/yunohost/domain.py @@ -388,6 +388,22 @@ def _get_conflicting_apps(domain, path, ignore_app=None): return conflicts +def _assert_no_conflicting_apps(domain, path, ignore_app=None): + + conflicts = _get_conflicting_apps(domain, path, ignore_app) + + if conflicts: + apps = [] + for path, app_id, app_label in conflicts: + apps.append(" * {domain:s}{path:s} → {app_label:s} ({app_id:s})".format( + domain=domain, + path=path, + app_id=app_id, + app_label=app_label, + )) + + raise YunohostError('app_location_unavailable', apps="\n".join(apps)) + def domain_url_available(domain, path): """ @@ -430,98 +446,6 @@ def _normalize_domain_path(domain, path): return domain, path -def _check_and_sanitize_permission_path(url, app_main_path, permission): - """ - Check and normalize the urls passed for all permissions - Also check that the Regex is valid - - As documented in the 'ynh_permission_create' helper: - - If provided, 'url' is 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 - domain.tld -> domain.tld - - 'url' can be later treated as a regex if it starts 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]*$ - """ - - domains = domain_list()['domains'] - - # regex without domain - if url.startswith('re:/'): - regex = url[4:] - # check regex if it's a PCRE regex, if it's a lua regex just print a warning - # I don't how how to validate a regex - if '%' in regex: - logger.warning("/!\\ Packagers! You are probably using a lua regex. You should use a PCRE regex instead.") - else: - try: - re.compile(regex) - except Exception: - raise YunohostError('invalid_regex', regex=regex) - return url - - # regex with domain - if url.startswith('re:'): - if '/' not in url: - raise YunohostError('regex_with_only_domain') - domain = url[3:].split('/')[0] - path = '/' + url[3:].split('/', 1)[1] - - if domain.replace('%', '').replace('\\', '') not in domains: - raise YunohostError('domain_name_unknown', domain=domain) - - if '%' in path: - logger.warning("/!\\ Packagers! You are probably using a lua regex. You should use a PCRE regex instead.") - else: - try: - re.compile(path) - except Exception: - raise YunohostError('invalid_regex', regex=path) - - return 're:' + domain + path - - # uris without domain - if url.startswith('/'): - sanitized_url = '/' + url.strip("/") - domain = app_main_path.split('/')[0] - path = ('/' + app_main_path.split('/')[1]) if '/' in app_main_path else '/' - - # uris with domain - else: - domain = url.split('/')[0] - if domain not in domains: - raise YunohostError('domain_name_unknown', domain=domain) - - if '/' in url: - path = '/' + url.split('/', 1)[1].rstrip('/') - sanitized_url = domain + path - else: - sanitized_url = domain - path = '/' - - conflicts = _get_conflicting_apps(domain, path, ignore_app=permission.split('.')[0]) - - if conflicts: - apps = [] - for path, app_id, app_label in conflicts: - apps.append(" * {domain:s}{path:s} → {app_label:s} ({app_id:s})".format( - domain=domain, - path=path, - app_id=app_id, - app_label=app_label, - )) - - raise YunohostError('app_location_unavailable', apps="\n".join(apps)) - - return sanitized_url - - def _build_dns_conf(domain, ttl=3600, include_empty_AAAA_if_no_ipv6=False): """ Internal function that will returns a data structure containing the needed diff --git a/src/yunohost/permission.py b/src/yunohost/permission.py index bc5b7586f..a94897860 100644 --- a/src/yunohost/permission.py +++ b/src/yunohost/permission.py @@ -24,6 +24,7 @@ Manage permissions """ +import re import copy import grp import random @@ -368,15 +369,15 @@ def permission_url(operation_logger, permission, """ from yunohost.app import app_setting from yunohost.utils.ldap import _get_ldap_interface - from yunohost.domain import _check_and_sanitize_permission_path ldap = _get_ldap_interface() # By default, manipulate main permission if "." not in permission: permission = permission + ".main" + app = permission.split('.')[0] + if url or add_url: - app = permission.split('.')[0] domain = app_setting(app, 'domain') path = app_setting(app, 'path') if domain is None or path is None: @@ -395,7 +396,7 @@ def permission_url(operation_logger, permission, if url is None: url = existing_permission["url"] else: - url = _check_and_sanitize_permission_path(url, app_main_path, permission) + url = _validate_and_sanitize_permission_url(url, app_main_path, app) if url.startswith('re:') and existing_permission['show_tile']: logger.warning(m18n.n('regex_incompatible_with_tile', regex=url, permission=permission)) @@ -409,7 +410,7 @@ def permission_url(operation_logger, permission, if ur in current_additional_urls: logger.warning(m18n.n('additional_urls_already_added', permission=permission, url=ur)) else: - ur = _check_and_sanitize_permission_path(ur, app_main_path, permission) + ur = _validate_and_sanitize_permission_url(ur, app_main_path, app) new_additional_urls += [ur] if remove_url: @@ -645,3 +646,100 @@ def _get_absolute_url(url, base_path): return 're:' + base_path.replace('.', '\\.') + url[3:] else: return url + + +def _validate_and_sanitize_permission_url(url, app_base_path, app): + """ + Check and normalize the urls passed for all permissions + Also check that the Regex is valid + + As documented in the 'ynh_permission_create' helper: + + If provided, 'url' is 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 + domain.tld -> domain.tld + + 'url' can be later treated as a regex if it starts 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.domain import domain_list, _assert_no_conflicting_apps + + + domains = domain_list()['domains'] + + # + # Regexes + # + + def validate_regex(regex): + if '%' in regex: + logger.warning("/!\\ Packagers! You are probably using a lua regex. You should use a PCRE regex instead.") + return + + try: + re.compile(regex) + except Exception: + raise YunohostError('invalid_regex', regex=regex) + + if url.startswith('re:'): + + # regex without domain + + if url.startswith('re:/'): + validate_regex(url[4:]) + return url + + # regex with domain + + if '/' not in url: + raise YunohostError('regex_with_only_domain') + domain, path = url[3:].split('/', 1) + path = '/' + path + + if domain.replace('%', '').replace('\\', '') not in domains: + raise YunohostError('domain_name_unknown', domain=domain) + + validate_regex(path) + + return 're:' + domain + path + + # + # "Regular" URIs + # + + def split_domain_path(url): + url = url.strip("/") + (domain, path) = url.split('/', 1) if "/" in url else (url, "/") + if path != "/": + path = "/" + path + return (domain, path) + + # uris without domain + if url.startswith('/'): + # if url is for example /admin/ + # we want sanitized_url to be: /admin + # and (domain, path) to be : (domain.tld, /app/admin) + sanitized_url = "/" + url.strip("/") + domain, path = split_domain_path(app_base_path) + path = "/" + path.strip("/") + sanitized_url.strip("/") + + # uris with domain + else: + # if url is for example domain.tld/wat/ + # we want sanitized_url to be: domain.tld/wat + # and (domain, path) to be : (domain.tld, /wat) + domain, path = split_domain_path(url) + sanitized_url = domain + path + + if domain not in domains: + raise YunohostError('domain_name_unknown', domain=domain) + + _assert_no_conflicting_apps(domain, path, ignore_app=app) + + return sanitized_url diff --git a/src/yunohost/tests/test_appurl.py b/src/yunohost/tests/test_appurl.py index a6d86d040..bbac69f82 100644 --- a/src/yunohost/tests/test_appurl.py +++ b/src/yunohost/tests/test_appurl.py @@ -5,7 +5,8 @@ from conftest import get_test_apps_dir from yunohost.utils.error import YunohostError from yunohost.app import app_install, app_remove -from yunohost.domain import _get_maindomain, domain_url_available, _normalize_domain_path, _check_and_sanitize_permission_path +from yunohost.domain import _get_maindomain, domain_url_available, _normalize_domain_path +from yunohost.permission import _validate_and_sanitize_permission_url # Get main domain maindomain = _get_maindomain() @@ -66,43 +67,43 @@ def test_registerurl_baddomain(): def test_normalize_permission_path(): # Relative path - assert _check_and_sanitize_permission_path("/wiki/", maindomain + '/path', 'test_permission.main') == "/wiki" - assert _check_and_sanitize_permission_path("/", maindomain + '/path', 'test_permission.main') == "/" - assert _check_and_sanitize_permission_path("//salut/", maindomain + '/path', 'test_permission.main') == "/salut" + assert _validate_and_sanitize_permission_url("/wiki/", maindomain + '/path', 'test_permission') == "/wiki" + assert _validate_and_sanitize_permission_url("/", maindomain + '/path', 'test_permission') == "/" + assert _validate_and_sanitize_permission_url("//salut/", maindomain + '/path', 'test_permission') == "/salut" # Full path - assert _check_and_sanitize_permission_path(maindomain + "/hey/", maindomain + '/path', 'test_permission.main') == maindomain + "/hey" - assert _check_and_sanitize_permission_path(maindomain + "//", maindomain + '/path', 'test_permission.main') == maindomain + "/" - assert _check_and_sanitize_permission_path(maindomain + "/", maindomain + '/path', 'test_permission.main') == maindomain + "/" + assert _validate_and_sanitize_permission_url(maindomain + "/hey/", maindomain + '/path', 'test_permission') == maindomain + "/hey" + assert _validate_and_sanitize_permission_url(maindomain + "//", maindomain + '/path', 'test_permission') == maindomain + "/" + assert _validate_and_sanitize_permission_url(maindomain + "/", maindomain + '/path', 'test_permission') == maindomain + "/" # Relative Regex - assert _check_and_sanitize_permission_path("re:/yolo.*/", maindomain + '/path', 'test_permission.main') == "re:/yolo.*/" - assert _check_and_sanitize_permission_path("re:/y.*o(o+)[a-z]*/bo\1y", maindomain + '/path', 'test_permission.main') == "re:/y.*o(o+)[a-z]*/bo\1y" + assert _validate_and_sanitize_permission_url("re:/yolo.*/", maindomain + '/path', 'test_permission') == "re:/yolo.*/" + assert _validate_and_sanitize_permission_url("re:/y.*o(o+)[a-z]*/bo\1y", maindomain + '/path', 'test_permission') == "re:/y.*o(o+)[a-z]*/bo\1y" # Full Regex - assert _check_and_sanitize_permission_path("re:" + maindomain + "/yolo.*/", maindomain + '/path', 'test_permission.main') == "re:" + maindomain + "/yolo.*/" - assert _check_and_sanitize_permission_path("re:" + maindomain + "/y.*o(o+)[a-z]*/bo\1y", maindomain + '/path', 'test_permission.main') == "re:" + maindomain + "/y.*o(o+)[a-z]*/bo\1y" + assert _validate_and_sanitize_permission_url("re:" + maindomain + "/yolo.*/", maindomain + '/path', 'test_permission') == "re:" + maindomain + "/yolo.*/" + assert _validate_and_sanitize_permission_url("re:" + maindomain + "/y.*o(o+)[a-z]*/bo\1y", maindomain + '/path', 'test_permission') == "re:" + maindomain + "/y.*o(o+)[a-z]*/bo\1y" def test_normalize_permission_path_with_bad_regex(): # Relative Regex with pytest.raises(YunohostError): - _check_and_sanitize_permission_path("re:/yolo.*[1-7]^?/", maindomain + '/path', 'test_permission.main') + _validate_and_sanitize_permission_url("re:/yolo.*[1-7]^?/", maindomain + '/path', 'test_permission') with pytest.raises(YunohostError): - _check_and_sanitize_permission_path("re:/yolo.*[1-7](]/", maindomain + '/path', 'test_permission.main') + _validate_and_sanitize_permission_url("re:/yolo.*[1-7](]/", maindomain + '/path', 'test_permission') # Full Regex with pytest.raises(YunohostError): - _check_and_sanitize_permission_path("re:" + maindomain + "/yolo?+/", maindomain + '/path', 'test_permission.main') + _validate_and_sanitize_permission_url("re:" + maindomain + "/yolo?+/", maindomain + '/path', 'test_permission') with pytest.raises(YunohostError): - _check_and_sanitize_permission_path("re:" + maindomain + "/yolo[1-9]**/", maindomain + '/path', 'test_permission.main') + _validate_and_sanitize_permission_url("re:" + maindomain + "/yolo[1-9]**/", maindomain + '/path', 'test_permission') def test_normalize_permission_path_with_unknown_domain(): with pytest.raises(YunohostError): - _check_and_sanitize_permission_path("shouldntexist.tld/hey", maindomain + '/path', 'test_permission.main') + _validate_and_sanitize_permission_url("shouldntexist.tld/hey", maindomain + '/path', 'test_permission') with pytest.raises(YunohostError): - _check_and_sanitize_permission_path("re:shouldntexist.tld/hey.*", maindomain + '/path', 'test_permission.main') + _validate_and_sanitize_permission_url("re:shouldntexist.tld/hey.*", maindomain + '/path', 'test_permission') def test_normalize_permission_path_conflicting_path(): @@ -110,6 +111,6 @@ def test_normalize_permission_path_conflicting_path(): args="domain=%s&path=%s" % (maindomain, "/url/registerapp"), force=True) with pytest.raises(YunohostError): - _check_and_sanitize_permission_path("/registerapp", maindomain + '/url', 'test_permission.main') + _validate_and_sanitize_permission_url("/registerapp", maindomain + '/url', 'test_permission') with pytest.raises(YunohostError): - _check_and_sanitize_permission_path(maindomain + "/url/registerapp", maindomain + '/path', 'test_permission.main') + _validate_and_sanitize_permission_url(maindomain + "/url/registerapp", maindomain + '/path', 'test_permission')