From b457e78acea813831c5a16e8e9d9515396c1b81e Mon Sep 17 00:00:00 2001 From: selfhoster1312 Date: Fri, 10 May 2024 17:33:19 +0200 Subject: [PATCH 1/8] Only query user_list once --- src/tests/test_sso_and_portalapi.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/tests/test_sso_and_portalapi.py b/src/tests/test_sso_and_portalapi.py index e5ad8992f..1b9c4d7d2 100644 --- a/src/tests/test_sso_and_portalapi.py +++ b/src/tests/test_sso_and_portalapi.py @@ -43,9 +43,10 @@ def setup_module(module): assert os.system("systemctl is-active yunohost-portal-api >/dev/null") == 0 - if "alice" not in user_list()["users"]: + userlist = user_list()["users"] + if "alice" not in userlist: user_create("alice", maindomain, dummy_password, fullname="Alice White", admin=True) - if "bob" not in user_list()["users"]: + if "bob" not in userlist: user_create("bob", maindomain, dummy_password, fullname="Bob Marley") app_install( @@ -56,10 +57,9 @@ def setup_module(module): def teardown_module(module): - if "alice" in user_list()["users"]: - user_delete("alice") - if "bob" in user_list()["users"]: - user_delete("bob") + userlist = user_list()["users"] + for user in [ "alice", "bob" ]: + if user in userlist: user_delete(user) app_remove("hellopy") From 3f070edb610f942a4f081dc87bad7cf8a9efaabe Mon Sep 17 00:00:00 2001 From: selfhoster1312 Date: Fri, 10 May 2024 17:33:53 +0200 Subject: [PATCH 2/8] Ensure secondarydomain/subdomain are configured, only list once --- src/tests/test_sso_and_portalapi.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/tests/test_sso_and_portalapi.py b/src/tests/test_sso_and_portalapi.py index 1b9c4d7d2..c2afa6107 100644 --- a/src/tests/test_sso_and_portalapi.py +++ b/src/tests/test_sso_and_portalapi.py @@ -49,6 +49,11 @@ def setup_module(module): if "bob" not in userlist: user_create("bob", maindomain, dummy_password, fullname="Bob Marley") + domainlist = domain_list()["domains"] + domains = [ domain for domain in [ subdomain, secondarydomain ] if domain not in domainlist ] + for domain in domains: + domain_add(domain) + app_install( os.path.join(get_test_apps_dir(), "hellopy_ynh"), args=f"domain={maindomain}&init_main_permission=visitors", @@ -63,11 +68,10 @@ def teardown_module(module): app_remove("hellopy") - if subdomain in domain_list()["domains"]: - domain_remove(subdomain) - if secondarydomain in domain_list()["domains"]: - domain_remove(secondarydomain) - + domainlist = domain_list()["domains"] + domains = [ domain for domain in [ subdomain, secondarydomain ] if domain in domainlist ] + for domain in domains: + domain_remove(domain) def login(session, logged_as, logged_on=None): From 5efd72a9d722aab6a6cb07e47841aa00a77af7c0 Mon Sep 17 00:00:00 2001 From: selfhoster1312 Date: Fri, 10 May 2024 17:37:46 +0200 Subject: [PATCH 3/8] Add users_add bulk operation --- src/tests/test_sso_and_portalapi.py | 11 +++-- src/user.py | 74 +++++++++++++++++++++++++---- 2 files changed, 72 insertions(+), 13 deletions(-) diff --git a/src/tests/test_sso_and_portalapi.py b/src/tests/test_sso_and_portalapi.py index c2afa6107..344d65814 100644 --- a/src/tests/test_sso_and_portalapi.py +++ b/src/tests/test_sso_and_portalapi.py @@ -7,7 +7,7 @@ import os from .conftest import message, raiseYunohostError, get_test_apps_dir from yunohost.domain import _get_maindomain, domain_add, domain_remove, domain_list -from yunohost.user import user_create, user_list, user_delete +from yunohost.user import user_create, user_list, user_delete, User, users_add from yunohost.authenticators.ldap_ynhuser import Authenticator, SESSION_FOLDER, short_hash from yunohost.app import app_install, app_remove, app_setting, app_ssowatconf, app_change_url from yunohost.permission import user_permission_list, user_permission_update @@ -44,10 +44,11 @@ def setup_module(module): assert os.system("systemctl is-active yunohost-portal-api >/dev/null") == 0 userlist = user_list()["users"] - if "alice" not in userlist: - user_create("alice", maindomain, dummy_password, fullname="Alice White", admin=True) - if "bob" not in userlist: - user_create("bob", maindomain, dummy_password, fullname="Bob Marley") + users_to_add = [ user for user in [ + User("alice", maindomain, dummy_password, fullname="Alice White", admin=True), + User("bob", maindomain, dummy_password, fullname="Bob Marley"), + ] if user.name not in userlist ] + users_add(users_to_add) domainlist = domain_list()["domains"] domains = [ domain for domain in [ subdomain, secondarydomain ] if domain not in domainlist ] diff --git a/src/user.py b/src/user.py index f8ead156f..c30b550a3 100644 --- a/src/user.py +++ b/src/user.py @@ -24,6 +24,7 @@ import random import subprocess import copy from logging import getLogger +from typing import List, Optional from moulinette import Moulinette, m18n from moulinette.utils.process import check_output @@ -49,6 +50,20 @@ FIELDS_FOR_IMPORT = { ADMIN_ALIASES = ["root", "admin", "admins", "webmaster", "postmaster", "abuse"] +class User: + def __init__( + self, + name: str, + domain: str, + password: str, + fullname: Optional[str] = None, + admin: bool = False, + ): + self.name = name + self.password = password + self.domain = domain + self.fullname = fullname + self.admin = admin def user_list(fields=None): from yunohost.utils.ldap import _get_ldap_interface @@ -131,6 +146,40 @@ def shellexists(shell): """Check if the provided shell exists and is executable.""" return os.path.isfile(shell) and os.access(shell, os.X_OK) +# Used in tests to create many users at once. +# The permissions are synchronized at the end of the entire operation. +@is_unit_operation() +def users_add( + operation_logger, + users: List[User], +): + hooks = [] + for user in users: + # Only force regen_conf on the last iteration + hooks.append(user_create(user.name, user.domain, user.password, fullname=user.fullname, admin=user.admin, do_regen_conf=False)) + + # Invalidate passwd and group to take user and group creation into account + subprocess.call(["nscd", "-i", "passwd"]) + subprocess.call(["nscd", "-i", "group"]) + + # Add new users to all_users group + user_group_update(groupname="all_users", add=[ user.name for user in users ], force=True, sync_perm=False) + + # Do we have new admins? + admins = [ user.name for user in users if user.admin ] + if len(admins) > 0: + user_group_update(groupname="admins", add=admins, sync_perm=False) + + from yunohost.permission import permission_sync_to_user + from yunohost.hook import hook_callback + + # Now we can sync the permissions + permission_sync_to_user() + + for hook in hooks: + # Trigger post_user_create hooks + hook_callback("post_user_create", args=[hook["YNH_USER_USERNAME"], hook["YNH_USER_PASSWORD"]], env=hook) + logger.success(m18n.n("user_created")) @is_unit_operation([("username", "user")]) def user_create( @@ -143,6 +192,7 @@ def user_create( admin=False, from_import=False, loginShell=None, + do_regen_conf=True, ): if not fullname or not fullname.strip(): raise YunohostValidationError( @@ -260,9 +310,6 @@ def user_create( except Exception as e: raise YunohostError("user_creation_failed", user=username, error=e) - # Invalidate passwd and group to take user and group creation into account - subprocess.call(["nscd", "-i", "passwd"]) - subprocess.call(["nscd", "-i", "group"]) try: # Attempt to create user home folder @@ -277,13 +324,8 @@ def user_create( except subprocess.CalledProcessError: logger.warning(f"Failed to protect /home/{username}", exc_info=1) - # Create group for user and add to group 'all_users' user_group_create(groupname=username, gid=uid, primary_group=True, sync_perm=False) - user_group_update(groupname="all_users", add=username, force=True, sync_perm=True) - if admin: - user_group_update(groupname="admins", add=username, sync_perm=True) - # Trigger post_user_create hooks env_dict = { "YNH_USER_USERNAME": username, "YNH_USER_MAIL": mail, @@ -292,6 +334,22 @@ def user_create( "YNH_USER_LASTNAME": lastname, } + # If do_regen_conf is False, it means we are in a higher operation that will + # finish synchronizing everything, then run the hooks... so we return early, + # transmitting the env_dict for further hook run. + if not do_regen_conf: + return env_dict + + # Invalidate passwd and group to take user and group creation into account + subprocess.call(["nscd", "-i", "passwd"]) + subprocess.call(["nscd", "-i", "group"]) + + # Create group for user and add to group 'all_users' + user_group_update(groupname="all_users", add=username, force=True, sync_perm=not admin) + if admin: + user_group_update(groupname="admins", add=username, sync_perm=True) + + # Trigger post_user_create hooks hook_callback("post_user_create", args=[username, mail], env=env_dict) # TODO: Send a welcome mail to user From 74dced9b005f9e4882f5bcc407f8f43c98eb811a Mon Sep 17 00:00:00 2001 From: selfhoster1312 Date: Fri, 10 May 2024 18:02:40 +0200 Subject: [PATCH 4/8] Add _cert_exists to check if certificate is already generated --- src/certificate.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/certificate.py b/src/certificate.py index f76876290..a510f0d31 100644 --- a/src/certificate.py +++ b/src/certificate.py @@ -627,6 +627,9 @@ def _prepare_certificate_signing_request(domain, key_file, output_folder): with open(csr_file, "wb") as f: f.write(crypto.dump_certificate_request(crypto.FILETYPE_PEM, csr)) +def _cert_exists(domain): bool + cert_file = os.path.join(CERT_FOLDER, domain, "crt.pem") + return os.path.isfile(cert_file) def _get_status(domain): cert_file = os.path.join(CERT_FOLDER, domain, "crt.pem") From 7a52066f5697b92037e8bb7ba5f41eb91334fc58 Mon Sep 17 00:00:00 2001 From: selfhoster1312 Date: Fri, 10 May 2024 18:10:52 +0200 Subject: [PATCH 5/8] Add domains_add bulk operation --- src/certificate.py | 12 ++++-- src/domain.py | 64 +++++++++++++++++++++++++---- src/tests/test_sso_and_portalapi.py | 5 +-- 3 files changed, 67 insertions(+), 14 deletions(-) diff --git a/src/certificate.py b/src/certificate.py index a510f0d31..6885ea029 100644 --- a/src/certificate.py +++ b/src/certificate.py @@ -122,7 +122,7 @@ def certificate_install(domain_list, force=False, no_checks=False, self_signed=F _certificate_install_letsencrypt(domain_list, force, no_checks) -def _certificate_install_selfsigned(domain_list, force=False): +def _certificate_install_selfsigned(domain_list, force=False, do_regen_conf=True): failed_cert_install = [] for domain in domain_list: operation_logger = OperationLogger( @@ -201,7 +201,7 @@ def _certificate_install_selfsigned(domain_list, force=False): _set_permissions(conf_file, "root", "root", 0o600) # Actually enable the certificate we created - _enable_certificate(domain, new_cert_folder) + _enable_certificate(domain, new_cert_folder, do_regen_conf=do_regen_conf) # Check new status indicate a recently created self-signed certificate status = _get_status(domain) @@ -627,7 +627,7 @@ def _prepare_certificate_signing_request(domain, key_file, output_folder): with open(csr_file, "wb") as f: f.write(crypto.dump_certificate_request(crypto.FILETYPE_PEM, csr)) -def _cert_exists(domain): bool +def _cert_exists(domain) -> bool: cert_file = os.path.join(CERT_FOLDER, domain, "crt.pem") return os.path.isfile(cert_file) @@ -726,7 +726,7 @@ def _set_permissions(path, user, group, permissions): chmod(path, permissions) -def _enable_certificate(domain, new_cert_folder): +def _enable_certificate(domain, new_cert_folder, do_regen_conf=True): logger.debug("Enabling the certificate for domain %s ...", domain) live_link = os.path.join(CERT_FOLDER, domain) @@ -744,6 +744,10 @@ def _enable_certificate(domain, new_cert_folder): os.symlink(new_cert_folder, live_link) + # We are in a higher operation such as domains_add for bulk manipulation + # that will take care of service / hooks later + if not do_regen_conf: return + logger.debug("Restarting services...") for service in ("dovecot", "metronome"): diff --git a/src/domain.py b/src/domain.py index 06ab85f78..0ead23a4d 100644 --- a/src/domain.py +++ b/src/domain.py @@ -240,10 +240,58 @@ def _get_parent_domain_of(domain, return_self=False, topest=False): return domain if return_self else None +def domains_regen(domains: List[str]): + for domain in domains: + _force_clear_hashes([f"/etc/nginx/conf.d/{domain}.conf"]) + + from yunohost.app import app_ssowatconf + from yunohost.service import _run_service_command + logger.debug("Restarting services...") + for service in ("dovecot", "metronome"): + # Ugly trick to not restart metronome if it's not installed or no domain configured for XMPP + if service == "metronome" and ( + os.system("dpkg --list | grep -q 'ii *metronome'") != 0 + or not glob("/etc/metronome/conf.d/*.cfg.lua") + ): + continue + _run_service_command("restart", service) + + if os.path.isfile("/etc/yunohost/installed"): + # regen nginx conf to be sure it integrates OCSP Stapling + # (We don't do this yet if postinstall is not finished yet) + # We also regenconf for postfix to propagate the SNI hash map thingy + regen_conf( + names=[ + "nginx", + "metronome", + "dnsmasq", + "postfix", + "rspamd", + "mdns", + "dovecot", + ] + ) + app_ssowatconf() + _run_service_command("reload", "nginx") + +# Used in tests to create many domains at once. +# The permissions/configuration are synchronized at the end of the entire operation. +@is_unit_operation() +def domains_add(operation_logger, domains: List[str]): + for domain in domains: + domain_add(domain, do_regen_conf=False) + + domains_regen(domains) + + from yunohost.hook import hook_callback + for domain in domains: + hook_callback("post_cert_update", args=[domain]) + hook_callback("post_domain_add", args=[domain]) + logger.success(m18n.n("domain_created")) @is_unit_operation(exclude=["dyndns_recovery_password"]) def domain_add( - operation_logger, domain, dyndns_recovery_password=None, ignore_dyndns=False + operation_logger, domain, dyndns_recovery_password=None, ignore_dyndns=False, do_regen_conf=True ): """ Create a custom domain @@ -258,7 +306,7 @@ def domain_add( from yunohost.app import app_ssowatconf from yunohost.utils.ldap import _get_ldap_interface from yunohost.utils.password import assert_password_is_strong_enough - from yunohost.certificate import _certificate_install_selfsigned + from yunohost.certificate import _certificate_install_selfsigned, _cert_exists from yunohost.utils.dns import is_yunohost_dyndns_domain if dyndns_recovery_password: @@ -306,7 +354,8 @@ def domain_add( domain=domain, recovery_password=dyndns_recovery_password ) - _certificate_install_selfsigned([domain], True) + if not _cert_exists(domain): + _certificate_install_selfsigned([domain], True, do_regen_conf=False) try: attr_dict = { @@ -323,7 +372,8 @@ def domain_add( domain_list_cache = [] # Don't regen these conf if we're still in postinstall - if os.path.exists("/etc/yunohost/installed"): + # or if we're in a higher operation that will take care of it, such as domains_add + if os.path.exists("/etc/yunohost/installed") and do_regen_conf: # Sometime we have weird issues with the regenconf where some files # appears as manually modified even though they weren't touched ... # There are a few ideas why this happens (like backup/restore nginx @@ -356,9 +406,9 @@ def domain_add( pass raise e - hook_callback("post_domain_add", args=[domain]) - - logger.success(m18n.n("domain_created")) + if do_regen_conf: + hook_callback("post_domain_add", args=[domain]) + logger.success(m18n.n("domain_created")) @is_unit_operation(exclude=["dyndns_recovery_password"]) diff --git a/src/tests/test_sso_and_portalapi.py b/src/tests/test_sso_and_portalapi.py index 344d65814..0049e9857 100644 --- a/src/tests/test_sso_and_portalapi.py +++ b/src/tests/test_sso_and_portalapi.py @@ -6,7 +6,7 @@ import os from .conftest import message, raiseYunohostError, get_test_apps_dir -from yunohost.domain import _get_maindomain, domain_add, domain_remove, domain_list +from yunohost.domain import _get_maindomain, domain_add, domain_remove, domain_list, domains_add from yunohost.user import user_create, user_list, user_delete, User, users_add from yunohost.authenticators.ldap_ynhuser import Authenticator, SESSION_FOLDER, short_hash from yunohost.app import app_install, app_remove, app_setting, app_ssowatconf, app_change_url @@ -52,8 +52,7 @@ def setup_module(module): domainlist = domain_list()["domains"] domains = [ domain for domain in [ subdomain, secondarydomain ] if domain not in domainlist ] - for domain in domains: - domain_add(domain) + domains_add(domains) app_install( os.path.join(get_test_apps_dir(), "hellopy_ynh"), From 1ad0979ace172b5d04f9ffdbbca63bcd3bd514e8 Mon Sep 17 00:00:00 2001 From: selfhoster1312 Date: Fri, 10 May 2024 19:02:25 +0200 Subject: [PATCH 6/8] Add domains_remove bulk operation --- src/domain.py | 19 +++++++++++++++++++ src/tests/test_sso_and_portalapi.py | 5 ++--- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/src/domain.py b/src/domain.py index 0ead23a4d..598fd9e5f 100644 --- a/src/domain.py +++ b/src/domain.py @@ -274,6 +274,20 @@ def domains_regen(domains: List[str]): app_ssowatconf() _run_service_command("reload", "nginx") +# Used in tests to delete many domains at once. +# The permissions/configuration are synchronized at the end of the entire operation. +@is_unit_operation() +def domains_remove(operation_logger, domains: List[str]): + for domain in domains: + domain_remove(domain, do_regen_conf=False) + + domains_regen(domains) + + from yunohost.hook import hook_callback + for domain in domains: + hook_callback("post_domain_remove", args=[domain]) + logger.success(m18n.n("domain_deleted")) + # Used in tests to create many domains at once. # The permissions/configuration are synchronized at the end of the entire operation. @is_unit_operation() @@ -419,6 +433,7 @@ def domain_remove( force=False, dyndns_recovery_password=None, ignore_dyndns=False, + do_regen_conf=True, ): """ Delete domains @@ -541,6 +556,10 @@ def domain_remove( rm(key_file, force=True) rm(f"{DOMAIN_SETTINGS_DIR}/{domain}.yml", force=True) + # We are in a bulk domains_remove so don't regen_conf immediately + if not do_regen_conf: + return + # Sometime we have weird issues with the regenconf where some files # appears as manually modified even though they weren't touched ... # There are a few ideas why this happens (like backup/restore nginx diff --git a/src/tests/test_sso_and_portalapi.py b/src/tests/test_sso_and_portalapi.py index 0049e9857..78e1cb278 100644 --- a/src/tests/test_sso_and_portalapi.py +++ b/src/tests/test_sso_and_portalapi.py @@ -6,7 +6,7 @@ import os from .conftest import message, raiseYunohostError, get_test_apps_dir -from yunohost.domain import _get_maindomain, domain_add, domain_remove, domain_list, domains_add +from yunohost.domain import _get_maindomain, domain_add, domain_remove, domain_list, domains_add, domains_remove from yunohost.user import user_create, user_list, user_delete, User, users_add from yunohost.authenticators.ldap_ynhuser import Authenticator, SESSION_FOLDER, short_hash from yunohost.app import app_install, app_remove, app_setting, app_ssowatconf, app_change_url @@ -70,8 +70,7 @@ def teardown_module(module): domainlist = domain_list()["domains"] domains = [ domain for domain in [ subdomain, secondarydomain ] if domain in domainlist ] - for domain in domains: - domain_remove(domain) + domains_remove(domains) def login(session, logged_as, logged_on=None): From 288d6b6b4781f53f11edf1760de04ebf2ba2864c Mon Sep 17 00:00:00 2001 From: selfhoster1312 Date: Fri, 10 May 2024 19:31:05 +0200 Subject: [PATCH 7/8] Add users_remove bulk operation --- src/tests/test_sso_and_portalapi.py | 6 ++--- src/user.py | 39 +++++++++++++++++++++++++---- 2 files changed, 37 insertions(+), 8 deletions(-) diff --git a/src/tests/test_sso_and_portalapi.py b/src/tests/test_sso_and_portalapi.py index 78e1cb278..35eec594a 100644 --- a/src/tests/test_sso_and_portalapi.py +++ b/src/tests/test_sso_and_portalapi.py @@ -7,7 +7,7 @@ import os from .conftest import message, raiseYunohostError, get_test_apps_dir from yunohost.domain import _get_maindomain, domain_add, domain_remove, domain_list, domains_add, domains_remove -from yunohost.user import user_create, user_list, user_delete, User, users_add +from yunohost.user import user_create, user_list, user_delete, User, users_add, users_remove from yunohost.authenticators.ldap_ynhuser import Authenticator, SESSION_FOLDER, short_hash from yunohost.app import app_install, app_remove, app_setting, app_ssowatconf, app_change_url from yunohost.permission import user_permission_list, user_permission_update @@ -63,8 +63,8 @@ def setup_module(module): def teardown_module(module): userlist = user_list()["users"] - for user in [ "alice", "bob" ]: - if user in userlist: user_delete(user) + users = [ user for user in [ "alice", "bob" ] if user in userlist ] + users_remove(users) app_remove("hellopy") diff --git a/src/user.py b/src/user.py index c30b550a3..843124c94 100644 --- a/src/user.py +++ b/src/user.py @@ -146,6 +146,28 @@ def shellexists(shell): """Check if the provided shell exists and is executable.""" return os.path.isfile(shell) and os.access(shell, os.X_OK) +# Used in tests to create many users at once. +# The permissions are synchronized at the end of the entire operation. +@is_unit_operation() +def users_remove( + operation_logger, + users: List[str], +): + + for username in users: + user_delete(username, do_regen_conf=False) + + from yunohost.permission import permission_sync_to_user + permission_sync_to_user() + + # Invalidate passwd to take user deletion into account + subprocess.call(["nscd", "-i", "passwd"]) + + from yunohost.hook import hook_callback + for username in users: + hook_callback("post_user_delete", args=[username, False]) + logger.success(m18n.n("user_deleted")) + # Used in tests to create many users at once. # The permissions are synchronized at the end of the entire operation. @is_unit_operation() @@ -360,7 +382,7 @@ def user_create( @is_unit_operation([("username", "user")]) -def user_delete(operation_logger, username, purge=False, from_import=False): +def user_delete(operation_logger, username, purge=False, from_import=False, do_regen_conf=True): from yunohost.hook import hook_callback from yunohost.utils.ldap import _get_ldap_interface from yunohost.authenticators.ldap_ynhuser import Authenticator as PortalAuth @@ -372,7 +394,6 @@ def user_delete(operation_logger, username, purge=False, from_import=False): if not from_import: operation_logger.start() - user_group_update("all_users", remove=username, force=True, sync_perm=False) for group, infos in user_group_list()["groups"].items(): if group == "all_users": continue @@ -385,7 +406,14 @@ def user_delete(operation_logger, username, purge=False, from_import=False): # epic bug happened somewhere else and only a partial removal was # performed...) if username in user_group_list()["groups"].keys(): - user_group_delete(username, force=True, sync_perm=True) + user_group_delete(username, force=True, sync_perm=False) + + PortalAuth.invalidate_all_sessions_for_user(username) + AdminAuth.invalidate_all_sessions_for_user(username) + + # Apparently ldap.remove uid removes from group all_users, but unless we have test we + # can't be too sure... so leave it here until we have tests for this! + user_group_update("all_users", remove=username, force=True, sync_perm=do_regen_conf) ldap = _get_ldap_interface() try: @@ -393,8 +421,9 @@ def user_delete(operation_logger, username, purge=False, from_import=False): except Exception as e: raise YunohostError("user_deletion_failed", user=username, error=e) - PortalAuth.invalidate_all_sessions_for_user(username) - AdminAuth.invalidate_all_sessions_for_user(username) + if not do_regen_conf: + return + # Invalidate passwd to take user deletion into account subprocess.call(["nscd", "-i", "passwd"]) From aeda36f61014aeb7a71bcb8dfd87b39fbc1054c6 Mon Sep 17 00:00:00 2001 From: selfhoster1312 Date: Fri, 10 May 2024 20:22:32 +0200 Subject: [PATCH 8/8] App install/remove now has sync_perm setting --- src/app.py | 7 +++++-- src/tests/test_sso_and_portalapi.py | 27 +++++++++++++++------------ 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/src/app.py b/src/app.py index b38812f1c..db86f1538 100644 --- a/src/app.py +++ b/src/app.py @@ -1025,6 +1025,7 @@ def app_install( args=None, no_remove_on_failure=False, force=False, + sync_perm=True, ): """ Install apps @@ -1201,6 +1202,7 @@ def app_install( label=manifest["name"], show_tile=False, protected=False, + sync_perm=sync_perm ) # Prepare env. var. to pass to script @@ -1377,7 +1379,7 @@ def app_install( @is_unit_operation() -def app_remove(operation_logger, app, purge=False, force_workdir=None): +def app_remove(operation_logger, app, purge=False, force_workdir=None, sync_perm=True): """ Remove app @@ -1476,7 +1478,8 @@ def app_remove(operation_logger, app, purge=False, force_workdir=None): else: logger.warning(m18n.n("app_not_properly_removed", app=app)) - permission_sync_to_user() + if sync_perm: + permission_sync_to_user() _assert_system_is_sane_for_app(manifest, "post") diff --git a/src/tests/test_sso_and_portalapi.py b/src/tests/test_sso_and_portalapi.py index 35eec594a..20009395a 100644 --- a/src/tests/test_sso_and_portalapi.py +++ b/src/tests/test_sso_and_portalapi.py @@ -43,6 +43,18 @@ def setup_module(module): assert os.system("systemctl is-active yunohost-portal-api >/dev/null") == 0 + domainlist = domain_list()["domains"] + domains = [ domain for domain in [ subdomain, secondarydomain ] if domain not in domainlist ] + domains_add(domains) + + # Install app first, permissions will be synced after users_add + app_install( + os.path.join(get_test_apps_dir(), "hellopy_ynh"), + args=f"domain={maindomain}&init_main_permission=visitors", + force=True, + sync_perm=False, + ) + userlist = user_list()["users"] users_to_add = [ user for user in [ User("alice", maindomain, dummy_password, fullname="Alice White", admin=True), @@ -50,24 +62,15 @@ def setup_module(module): ] if user.name not in userlist ] users_add(users_to_add) - domainlist = domain_list()["domains"] - domains = [ domain for domain in [ subdomain, secondarydomain ] if domain not in domainlist ] - domains_add(domains) - - app_install( - os.path.join(get_test_apps_dir(), "hellopy_ynh"), - args=f"domain={maindomain}&init_main_permission=visitors", - force=True, - ) - def teardown_module(module): + # Remove app first, permissions will be synced after users_remove + app_remove("hellopy", sync_perm=False) + userlist = user_list()["users"] users = [ user for user in [ "alice", "bob" ] if user in userlist ] users_remove(users) - app_remove("hellopy") - domainlist = domain_list()["domains"] domains = [ domain for domain in [ subdomain, secondarydomain ] if domain in domainlist ] domains_remove(domains)