We probably don't need to have multiple urls per permissions ...

This commit is contained in:
Alexandre Aubin 2019-10-09 22:08:12 +02:00
parent 9cecd71437
commit 88794805eb
8 changed files with 105 additions and 147 deletions

View file

@ -232,34 +232,36 @@ ynh_webpath_register () {
# Create a new permission for the app
#
# example: ynh_permission_create --permission admin --urls /admin
# example: ynh_permission_create --permission admin --url /admin
#
# usage: ynh_permission_create --permission "permission" [--urls "url" ["url" ...]]
# usage: ynh_permission_create --permission "permission" [--url "url"]
# | arg: permission - the name for the permission (by default a permission named "main" already exist)
# | arg: urls - (optional) a list of FULL urls for the permission (e.g. domain.tld/apps/admin)
# | arg: urls - (optional) a list of URLs to specify for the permission.
# | arg: url - (optional) URL for which access will be allowed/forbidden
#
# 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
# 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
#
# URLs can be 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]*$
# '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]*$
#
ynh_permission_create() {
declare -Ar args_array=( [p]=permission= [u]=urls= )
declare -Ar args_array=( [p]=permission= [u]=url= )
local permission
local urls
ynh_handle_getopts_args "$@"
if [[ -n ${urls:-} ]]; then
urls=",urls=['${urls//';'/"','"}']"
if [[ -n ${url:-} ]]; then
url="'$url'"
else
url="None"
fi
yunohost tools shell -c "from yunohost.permission import permission_create; permission_create('$app.$permission' ${urls:-}, sync_perm=False)"
yunohost tools shell -c "from yunohost.permission import permission_create; permission_create('$app.$permission', url=$url, sync_perm=False)"
}
# Remove a permission for the app (note that when the app is removed all permission is automatically removed)
@ -277,30 +279,28 @@ ynh_permission_delete() {
yunohost tools shell -c "from yunohost.permission import permission_delete; permission_delete('$app.$permission', sync_perm=False)"
}
# Manage urls related to a permission
# Redefine the url associated to a permission
#
# usage: ynh_permission_urls --permission "permission" --add "url" ["url" ...] --remove "url" ["url" ...]
# usage: ynh_permission_url --permission "permission" --url "url"
# | arg: permission - the name for the permission (by default a permission named "main" is removed automatically when the app is removed)
# | arg: add - (optional) a list of urls to add to the permission (see permission_create for details regarding their format)
# | arg: remove - (optional) a list of urls to remove from the permission (see permission_create for details regarding their format)
# | arg: url - (optional) URL for which access will be allowed/forbidden
#
ynh_permission_urls() {
declare -Ar args_array=([p]=permission= [a]=add= [r]=remove=)
ynh_permission_url() {
declare -Ar args_array=([p]=permission= [u]=url=)
local permission
local add
local remove
local url
ynh_handle_getopts_args "$@"
if [[ -n ${add:-} ]]; then
add=",add=['${add//';'/"','"}']"
fi
if [[ -n ${remove:-} ]]; then
remove=",remove=['${remove//';'/"','"}']"
if [[ -n ${url:-} ]]; then
url="'$url'"
else
url="None"
fi
yunohost tools shell -c "from yunohost.permission import permission_urls; permission_urls('$app.$permission' ${add:-} ${remove:-})"
yunohost tools shell -c "from yunohost.permission import permission_url; permission_url('$app.$permission', url=$url)"
}
# Update a permission for the app
#
# usage: ynh_permission_update --permission "permission" --add "group" ["group" ...] --remove "group" ["group" ...]

View file

@ -268,7 +268,7 @@
"log_letsencrypt_cert_install": "Install a Let's encrypt certificate on '{}' domain",
"log_permission_create": "Create permission '{}'",
"log_permission_delete": "Delete permission '{}'",
"log_permission_urls": "Update urls related to permission '{}'",
"log_permission_url": "Update url related to permission '{}'",
"log_selfsigned_cert_install": "Install self signed certificate on '{}' domain",
"log_letsencrypt_cert_renew": "Renew '{}' Let's encrypt certificate",
"log_regen_conf": "Regenerate system configurations '{}'",

View file

@ -454,33 +454,18 @@ def app_map(app=None, raw=False, user=None):
return perm_domain, perm_path
this_app_perms = {p: i for p, i in permissions.items() if p.startswith(app_id + ".") and i["urls"]}
this_app_perms = {p: i for p, i in permissions.items() if p.startswith(app_id + ".") and i["url"]}
for perm_name, perm_info in this_app_perms.items():
# If we're building the map for a specific user, check the user
# actually is allowed for this specific perm
if user and user not in perm_info["corresponding_users"] and "visitors" not in perm_info["allowed"]:
continue
if len(perm_info["urls"]) > 1 or perm_info["urls"][0].startswith("re:"):
#
# Here we have a big conceptual issue about the sso ...
# Let me take a sip of coffee and turn off the music...
#
# Let's say we have an app foo which created a permission
# 'foo.admin' and added as url "/admin" and "/api" This
# permission got defined somehow as only accessible for group
# "admins". So both "/admin" and "/api" are protected. Good!
#
# Now if we really want users in group "admins" to access those
# uris, then each users in group "admins" need to have these
# urls in the ssowat dict for this user. Which corresponds to a
# tile. To put it otherwise : in the current code of ssowat, a
# permission = a tile = a url !
#
# We also have an issue if the url define is a regex, because
if perm_info["url"].startswith("re:"):
# Here, we have an issue if the chosen url is a regex, because
# the url we want to add to the dict is going to be turned into
# a clickable link (or analyzed by other parts of yunohost
# code...). To put it otherwise : in the current code of ssowat,
# you can't give access a user to a regex
# you can't give access a user to a regex.
#
# Instead, as drafted by Josue, we could rework the ssowat logic
# about how routes and their permissions are defined. So for example,
@ -498,10 +483,10 @@ def app_map(app=None, raw=False, user=None):
# protected/unprotected/skipped uris and regexes and we gotta
# handle / migrate all the legacy stuff somehow if we don't
# want to end up with a total mess in the future idk
logger.error("Permission %s can't be added to the SSOwat configuration because it uses multiple urls and/or uses a regex url" % perm_name)
logger.error("Permission %s can't be added to the SSOwat configuration because it doesn't support regexes so far..." % perm_name)
continue
perm_domain, perm_path = _sanitized_absolute_url(perm_info["urls"][0])
perm_domain, perm_path = _sanitized_absolute_url(perm_info["url"])
if perm_name.endswith(".main"):
perm_label = label
@ -535,7 +520,6 @@ 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.permission import permission_urls
installed = _is_installed(app)
if not installed:
@ -835,7 +819,7 @@ def app_install(operation_logger, app, label=None, args=None, no_remove_on_failu
from yunohost.hook import hook_add, hook_remove, hook_exec, hook_callback
from yunohost.log import OperationLogger
from yunohost.permission import user_permission_list, permission_create, permission_urls, permission_delete, permission_sync_to_user, user_permission_update
from yunohost.permission import user_permission_list, permission_create, permission_url, permission_delete, permission_sync_to_user, user_permission_update
# Fetch or extract sources
if not os.path.exists(INSTALL_TMP):
@ -994,7 +978,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", urls=["/"])
permission_create(app_instance_name+".main", url="/")
# Execute the app install script
install_retcode = 1
@ -1088,7 +1072,7 @@ def app_install(operation_logger, app, label=None, args=None, no_remove_on_failu
domain = app_settings.get('domain', None)
path = app_settings.get('path', None)
if not (domain and path):
permission_urls(app_instance_name + ".main", remove=["/"], sync_perm=False)
permission_url(app_instance_name + ".main", url=None, sync_perm=False)
# Migrate classic public app still using the legacy unprotected_uris
if app_settings.get("unprotected_uris", None) == "/":
@ -1178,7 +1162,7 @@ def app_addaccess(apps, users=[]):
"""
from yunohost.permission import user_permission_update
logger.warning("/!\\ Packagers ! This app is using the legacy permission system. Please use the new helpers ynh_permission_{create,urls,update,delete} and the 'visitors' group to manage permissions.")
logger.warning("/!\\ Packagers ! This app is using the legacy permission system. Please use the new helpers ynh_permission_{create,url,update,delete} and the 'visitors' group to manage permissions.")
output = {}
for app in apps:
@ -1199,7 +1183,7 @@ def app_removeaccess(apps, users=[]):
"""
from yunohost.permission import user_permission_update
logger.warning("/!\\ Packagers ! This app is using the legacy permission system. Please use the new helpers ynh_permission_{create,urls,update,delete} and the 'visitors' group to manage permissions.")
logger.warning("/!\\ Packagers ! This app is using the legacy permission system. Please use the new helpers ynh_permission_{create,url,update,delete} and the 'visitors' group to manage permissions.")
output = {}
for app in apps:
@ -1219,7 +1203,7 @@ def app_clearaccess(apps):
"""
from yunohost.permission import user_permission_reset
logger.warning("/!\\ Packagers ! This app is using the legacy permission system. Please use the new helpers ynh_permission_{create,urls,update,delete} and the 'visitors' group to manage permissions.")
logger.warning("/!\\ Packagers ! This app is using the legacy permission system. Please use the new helpers ynh_permission_{create,url,update,delete} and the 'visitors' group to manage permissions.")
output = {}
for app in apps:
@ -1329,7 +1313,7 @@ def app_setting(app, key, value=None, delete=False):
if key in ['redirected_urls', 'redirected_regex']:
value = yaml.load(value)
if key in ["unprotected_uris", "unprotected_regex", "protected_uris", "protected_regex"]:
logger.warning("/!\ Packagers ! This app is using the legacy permission system. Please delete these legacy settings and use the new helpers ynh_permission_{create,urls,update,delete} and the 'visitors' group to manage public/private access.")
logger.warning("/!\ Packagers ! This app is using the legacy permission system. Please delete these legacy settings and use the new helpers ynh_permission_{create,url,update,delete} and the 'visitors' group to manage public/private access.")
app_settings[key] = value
_set_app_settings(app, app_settings)
@ -1562,20 +1546,24 @@ def app_ssowatconf():
# New permission system
this_app_perms = {name: info for name, info in all_permissions.items() if name.startswith(app['id'] + ".")}
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
# FIXME : gotta handle regex-urls here... meh
urls = [_sanitized_absolute_url(url) for url in perm_info["urls"]]
url = _sanitized_absolute_url(perm_info["url"])
if "visitors" in perm_info["allowed"]:
unprotected_urls += urls
unprotected_urls.append(url)
# Legacy stuff : we remove now unprotected-urls that might have been declared as protected earlier...
protected_urls = [u for u in protected_urls if u not in urls]
protected_urls = [u for u in protected_urls if u != url]
else:
# TODO : small optimization to implement : we don't need to explictly add all the app roots
protected_urls += urls
protected_urls.append(url)
# Legacy stuff : we remove now unprotected-urls that might have been declared as protected earlier...
unprotected_urls = [u for u in unprotected_urls if u not in urls]
unprotected_urls = [u for u in unprotected_urls if u != url]
for domain in domains:
skipped_urls.extend([domain + '/yunohost/admin', domain + '/yunohost/api'])
@ -1585,11 +1573,13 @@ def app_ssowatconf():
skipped_regex.append("^[^/]*/%.well%-known/autoconfig/mail/config%-v1%.1%.xml.*$")
permissions_per_url = {}
for permission_name, permission_infos in all_permissions.items():
for url in permission_infos["urls"]:
permissions_per_url[url] = permission_infos['corresponding_users']
for perm_name, perm_info in all_permissions.items():
# Ignore permissions for which there's no url defined
if not perm_info["url"]:
continue
permissions_per_url[perm_info["url"]] = perm_info['corresponding_users']
conf_dict = {
'portal_domain': main_domain,

View file

@ -1189,7 +1189,7 @@ class RestoreManager():
return
from yunohost.user import user_group_list
from yunohost.permission import permission_create, permission_delete, user_permission_update, user_permission_list
from yunohost.permission import permission_create, permission_delete, user_permission_update, user_permission_list, permission_sync_to_user
# Backup old permission for apps
# We need to do that because in case of an app is installed we can't remove the permission for this app
@ -1251,7 +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, urls=permission_infos["urls"], sync_perm=False)
permission_create(permission_name, url=permission_infos["url"], sync_perm=False)
user_permission_update(permission_name, remove="all_users", add=permission_infos["allowed"])
def _restore_apps(self):
@ -1362,7 +1362,7 @@ class RestoreManager():
for permission_name, permission_infos in permissions.items():
permission_create(permission_name, urls=permission_infos.get("urls", []))
permission_create(permission_name, url=permission_infos.get("url", None))
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))

View file

@ -107,8 +107,8 @@ class MyMigration(Migration):
path = app_setting(app, 'path')
domain = app_setting(app, 'domain')
urls = "/" if domain and path else None
permission_create(app+".main", urls=urls, sync_perm=False)
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)

View file

@ -73,7 +73,7 @@ def user_permission_list(short=False, full=False, ignore_system_perms=False):
if full:
permissions[name]["corresponding_users"] = [_ldap_path_extract(p, "uid") for p in infos.get('inheritPermission', [])]
permissions[name]["urls"] = infos.get("URL", [])
permissions[name]["url"] = infos.get("URL", [None])[0]
if short:
permissions = permissions.keys()
@ -260,27 +260,27 @@ def user_permission_reset(operation_logger, permission, sync_perm=True):
#
# The followings methods are *not* directly exposed.
# They are used to create/delete the permissions (e.g. during app install/remove)
# and by some app helpers to possibly add additional permissions and tweak the urls
# and by some app helpers to possibly add additional permissions
#
#
@is_unit_operation()
def permission_create(operation_logger, permission, urls=None, sync_perm=True):
def permission_create(operation_logger, permission, url=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)
urls -- list of URLs to specify for the permission.
url -- (optional) URL for which access will be allowed/forbidden
Urls are assumed to be relative to the app domain/path if they start with '/'.
For example:
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
URLs can be later treated as regexes when they start with "re:".
'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]*$
@ -316,8 +316,8 @@ def permission_create(operation_logger, permission, urls=None, sync_perm=True):
if permission.endswith(".main"):
attr_dict['groupPermission'] = ['cn=all_users,ou=groups,dc=yunohost,dc=org']
if urls:
attr_dict['URL'] = urls
if url:
attr_dict['URL'] = url
operation_logger.related_to.append(('app', permission.split(".")[0]))
operation_logger.start()
@ -335,15 +335,13 @@ def permission_create(operation_logger, permission, urls=None, sync_perm=True):
@is_unit_operation()
def permission_urls(operation_logger, permission, add=None, remove=None, sync_perm=True):
def permission_url(operation_logger, permission, url=None, sync_perm=True):
"""
Update urls related to a permission for a specific application
Keyword argument:
permission -- Name of the permission (e.g. mail or nextcloud or wordpress.editors)
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)
url -- (optional) URL for which access will be allowed/forbidden
"""
from yunohost.utils.ldap import _get_ldap_interface
ldap = _get_ldap_interface()
@ -355,17 +353,9 @@ def permission_urls(operation_logger, permission, add=None, remove=None, sync_pe
raise YunohostError('permission_not_found', permission=permission)
# Compute new url list
old_url = existing_permission["url"]
new_urls = copy.copy(existing_permission["urls"])
if add:
urls_to_add = [add] if not isinstance(add, list) else add
new_urls += urls_to_add
if remove:
urls_to_remove = [remove] if not isinstance(remove, list) else remove
new_urls = [u for u in new_urls if u not in urls_to_remove]
if set(new_urls) == set(existing_permission["urls"]):
if old_url == url:
logger.warning(m18n.n('permission_update_nothing_to_do'))
return existing_permission
@ -375,7 +365,7 @@ def permission_urls(operation_logger, permission, add=None, remove=None, sync_pe
operation_logger.start()
try:
ldap.update('cn=%s,ou=permission' % permission, {'URL': new_urls})
ldap.update('cn=%s,ou=permission' % permission, {'URL': [url]})
except Exception as e:
raise YunohostError('permission_update_failed', permission=permission, error=e)

View file

@ -529,11 +529,11 @@ def test_backup_and_restore_permission_app():
assert "permissions_app.main" in res
assert "permissions_app.admin" in res
assert "permissions_app.dev" in res
assert res['permissions_app.main']['urls'] == ["/"]
assert res['permissions_app.admin']['urls'] == ["/admin"]
assert res['permissions_app.dev']['urls'] == ["/dev"]
assert res['permissions_app.main']['url'] == "/"
assert res['permissions_app.admin']['url'] == "/admin"
assert res['permissions_app.dev']['url'] == "/dev"
assert res['permissions_app.main']['allowed'] == ["all_users"]
assert res['permissions_app.main']['allowed'] == ["visitors"]
assert res['permissions_app.admin']['allowed'] == ["alice"]
assert res['permissions_app.dev']['allowed'] == []
@ -543,11 +543,11 @@ def test_backup_and_restore_permission_app():
assert "permissions_app.main" in res
assert "permissions_app.admin" in res
assert "permissions_app.dev" in res
assert res['permissions_app.main']['urls'] == ["/"]
assert res['permissions_app.admin']['urls'] == ["/admin"]
assert res['permissions_app.dev']['urls'] == ["/dev"]
assert res['permissions_app.main']['url'] == "/"
assert res['permissions_app.admin']['url'] == "/admin"
assert res['permissions_app.dev']['url'] == "/dev"
assert res['permissions_app.main']['allowed'] == ["all_users"]
assert res['permissions_app.main']['allowed'] == ["visitors"]
assert res['permissions_app.admin']['allowed'] == ["alice"]
assert res['permissions_app.dev']['allowed'] == []

View file

@ -6,7 +6,7 @@ from yunohost.app import app_install, app_remove, app_change_url, app_list, app_
from yunohost.user import user_list, user_info, user_create, user_delete, user_update, \
user_group_list, user_group_create, user_group_delete, user_group_update, user_group_info
from yunohost.permission import user_permission_update, user_permission_list, user_permission_reset, \
permission_create, permission_urls, permission_delete
permission_create, permission_delete, permission_url
from yunohost.domain import _get_maindomain
from yunohost.utils.error import YunohostError
@ -31,7 +31,7 @@ def setup_function(function):
user_create("alice", "Alice", "White", "alice@" + maindomain, dummy_password)
user_create("bob", "Bob", "Snow", "bob@" + maindomain, dummy_password)
permission_create("wiki.main", urls=["/"], sync_perm=False)
permission_create("wiki.main", url="/", sync_perm=False)
permission_create("blog.main", sync_perm=False)
user_permission_update("blog.main", remove="all_users", add="alice")
@ -202,7 +202,7 @@ def test_permission_list():
assert res['blog.main']['allowed'] == ["alice"]
assert set(res['wiki.main']['corresponding_users']) == set(["alice", "bob"])
assert res['blog.main']['corresponding_users'] == ["alice"]
assert res['wiki.main']['urls'] == ["/"]
assert res['wiki.main']['url'] == "/"
#
# Create - Remove functions
@ -333,41 +333,19 @@ def test_permission_update_permission_that_doesnt_exist():
with pytest.raises(YunohostError):
user_permission_update("doesnt.exist", add="alice")
# Permission url management
def test_permission_add_url():
permission_urls("blog.main", add=["/testA"])
def test_permission_redefine_url():
permission_url("blog.main", url="/pwet")
res = user_permission_list(full=True)['permissions']
assert res["blog.main"]["urls"] == ["/testA"]
def test_permission_add_another_url():
permission_urls("wiki.main", add=["/testA"])
res = user_permission_list(full=True)['permissions']
assert set(res["wiki.main"]["urls"]) == set(["/", "/testA"])
assert res["blog.main"]["url"] == "/pwet"
def test_permission_remove_url():
permission_urls("wiki.main", remove=["/"])
permission_url("blog.main", url=None)
res = user_permission_list(full=True)['permissions']
assert res["wiki.main"]["urls"] == []
def test_permission_add_url_already_added():
res = user_permission_list(full=True)['permissions']
assert res["wiki.main"]["urls"] == ["/"]
permission_urls("wiki.main", add=["/"])
res = user_permission_list(full=True)['permissions']
assert res["wiki.main"]["urls"] == ["/"]
def test_permission_remove_url_not_added():
permission_urls("wiki.main", remove=["/doesnt_exist"])
res = user_permission_list(full=True)['permissions']
assert res['wiki.main']['urls'] == ["/"]
assert res["blog.main"]["url"] is None
#
# Application interaction
@ -381,9 +359,9 @@ def test_permission_app_install():
assert "permissions_app.main" in res
assert "permissions_app.admin" in res
assert "permissions_app.dev" in res
assert res['permissions_app.main']['urls'] == ["/"]
assert res['permissions_app.admin']['urls'] == ["/admin"]
assert res['permissions_app.dev']['urls'] == ["/dev"]
assert res['permissions_app.main']['url'] == "/"
assert res['permissions_app.admin']['url'] == "/admin"
assert res['permissions_app.dev']['url'] == "/dev"
assert res['permissions_app.main']['allowed'] == ["all_users"]
assert set(res['permissions_app.main']['corresponding_users']) == set(["alice", "bob"])
@ -416,16 +394,16 @@ def test_permission_app_change_url():
# FIXME : should rework this test to look for differences in the generated app map / app tiles ...
res = user_permission_list(full=True)['permissions']
assert res['permissions_app.main']['urls'] == ["/"]
assert res['permissions_app.admin']['urls'] == ["/admin"]
assert res['permissions_app.dev']['urls'] == ["/dev"]
assert res['permissions_app.main']['url'] == "/"
assert res['permissions_app.admin']['url'] == "/admin"
assert res['permissions_app.dev']['url'] == "/dev"
app_change_url("permissions_app", maindomain, "/newchangeurl")
res = user_permission_list(full=True)['permissions']
assert res['permissions_app.main']['urls'] == ["/"]
assert res['permissions_app.admin']['urls'] == ["/admin"]
assert res['permissions_app.dev']['urls'] == ["/dev"]
assert res['permissions_app.main']['url'] == "/"
assert res['permissions_app.admin']['url'] == "/admin"
assert res['permissions_app.dev']['url'] == "/dev"
def test_permission_app_propagation_on_ssowat():