From a044f3ad7ea87bbeb12fefccae1804d59ce55ac0 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Mon, 2 Dec 2019 19:04:07 +0100 Subject: [PATCH] Implicitly add all_users when adding visitors group --- locales/en.json | 3 ++- src/yunohost/permission.py | 12 +++++++++--- src/yunohost/tests/test_permission.py | 21 +++++++++++++++++++++ 3 files changed, 32 insertions(+), 4 deletions(-) diff --git a/locales/en.json b/locales/en.json index d93d2b5f4..17014cad0 100644 --- a/locales/en.json +++ b/locales/en.json @@ -415,14 +415,15 @@ "pattern_positive_number": "Must be a positive number", "pattern_username": "Must be lower-case alphanumeric and underscore characters only", "pattern_password_app": "Sorry, passwords can not contain the following characters: {forbidden_chars}", + "permission_all_users_implicitly_added": "The permission was also implicitly granted to 'all_users' because it is required to allow the special group 'visitors'", "permission_already_allowed": "Group '{group}' already has permission '{permission}' enabled", "permission_already_disallowed": "Group '{group}' already has permission '{permission}' disabled'", "permission_already_exist": "Permission '{permission}' already exists", "permission_already_up_to_date": "The permission was not updated because the addition/removal requests already match the current state.", + "permission_cannot_remove_all_users_while_visitors_allowed": "You can't remove this permission for 'all_users' while it is still allowed for 'visitors'", "permission_cannot_remove_main": "Removing a main permission is not allowed", "permission_created": "Permission '{permission:s}' created", "permission_creation_failed": "Could not create permission '{permission}': {error}", - "permission_allowed_for_visitors_but_not_for_all_users": "Visitors can't be granted if all users is not already granted.", "permission_currently_allowed_for_all_users": "This permission is currently granted to all users in addition to other groups. You probably want to either remove the 'all_users' permission or remove the other groups it is currently granted to.", "permission_deleted": "Permission '{permission:s}' deleted", "permission_deletion_failed": "Could not delete permission '{permission}': {error}", diff --git a/src/yunohost/permission.py b/src/yunohost/permission.py index b4abf4ca7..9cc7c7534 100644 --- a/src/yunohost/permission.py +++ b/src/yunohost/permission.py @@ -146,9 +146,15 @@ def user_permission_update(operation_logger, permission, add=None, remove=None, if "visitors" not in new_allowed_groups or len(new_allowed_groups) >= 3: logger.warning(m18n.n("permission_currently_allowed_for_all_users")) - # If visitors are allowed, but not all users, it can break some applications, so we prohibit it. - if "visitors" in new_allowed_groups and "all_users" not in new_allowed_groups: - raise YunohostError('permission_allowed_for_visitors_but_not_for_all_users') + # If visitors are to be added, we shall make sure that "all_users" are also allowed + # (e.g. if visitors are allowed to visit nextcloud, you still want to allow people to log in ...) + if add and "visitors" in groups_to_add and "all_users" not in new_allowed_groups: + new_allowed_groups.append("all_users") + logger.warning(m18n.n("permission_all_users_implicitly_added")) + # If all_users are to be added, yet visitors are still to allowed, then we + # refuse it (c.f. previous comment...) + if remove and "all_users" in groups_to_remove and "visitors" in new_allowed_groups: + raise YunohostError('permission_cannot_remove_all_users_while_visitors_allowed') # Don't update LDAP if we update exactly the same values if set(new_allowed_groups) == set(current_allowed_groups): diff --git a/src/yunohost/tests/test_permission.py b/src/yunohost/tests/test_permission.py index a4bd570e5..2e53f13a7 100644 --- a/src/yunohost/tests/test_permission.py +++ b/src/yunohost/tests/test_permission.py @@ -313,6 +313,27 @@ def test_permission_add_and_remove_group(mocker): assert res['wiki.main']['corresponding_users'] == ["alice"] +def test_permission_adding_visitors_implicitly_add_all_users(mocker): + + res = user_permission_list(full=True)['permissions'] + assert res['blog.main']['allowed'] == ["alice"] + + with message(mocker, "permission_updated", permission="blog.main"): + user_permission_update("blog.main", add="visitors") + + res = user_permission_list(full=True)['permissions'] + assert set(res['blog.main']['allowed']) == set(["alice", "visitors", "all_users"]) + + +def test_permission_cant_remove_all_users_if_visitors_allowed(mocker): + + with message(mocker, "permission_updated", permission="blog.main"): + user_permission_update("blog.main", add=["visitors", "all_users"]) + + with raiseYunohostError(mocker, 'permission_cannot_remove_all_users_while_visitors_allowed'): + user_permission_update("blog.main", remove="all_users") + + def test_permission_add_group_already_allowed(mocker): with message(mocker, "permission_already_allowed", permission="blog.main", group="alice"): user_permission_update("blog.main", add="alice")