Merge aeda36f610 into 54fd311bec

src/app.py

@@ -1033,6 +1033,7 @@ def app_install(
     args=None,
     no_remove_on_failure=False,
     force=False,
+    sync_perm=True,
 ):
     """
     Install apps
@@ -1209,6 +1210,7 @@ def app_install(
             label=manifest["name"],
             show_tile=False,
             protected=False,
+            sync_perm=sync_perm
         )
 
     # Prepare env. var. to pass to script
@@ -1385,7 +1387,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
 
@@ -1487,7 +1489,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")
 
 

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)
@@ -609,6 +609,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")
@@ -705,7 +708,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)
@@ -723,6 +726,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...")
 
     if os.path.isfile("/etc/yunohost/installed"):

src/domain.py

@@ -240,10 +240,72 @@ 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 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()
+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 +320,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:
@@ -300,7 +362,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 = {
@@ -317,7 +380,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
@@ -348,9 +412,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"])
@@ -361,6 +425,7 @@ def domain_remove(
     force=False,
     dyndns_recovery_password=None,
     ignore_dyndns=False,
+    do_regen_conf=True,
 ):
     """
     Delete domains
@@ -483,6 +548,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

src/tests/test_sso_and_portalapi.py

@@ -6,8 +6,8 @@ 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.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, 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
@@ -43,31 +43,37 @@ def setup_module(module):
 
     assert os.system("systemctl is-active yunohost-portal-api >/dev/null") == 0
 
-    if "alice" not in user_list()["users"]:
-        user_create("alice", maindomain, dummy_password, fullname="Alice White", admin=True)
-    if "bob" not in user_list()["users"]:
-        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 ]
+    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),
+        User("bob", maindomain, dummy_password, fullname="Bob Marley"),
+    ] if user.name not in userlist ]
+    users_add(users_to_add)
+
 
 def teardown_module(module):
-    if "alice" in user_list()["users"]:
-        user_delete("alice")
-    if "bob" in user_list()["users"]:
-        user_delete("bob")
+    # Remove app first, permissions will be synced after users_remove
+    app_remove("hellopy", sync_perm=False)
 
-    app_remove("hellopy")
-
-    if subdomain in domain_list()["domains"]:
-        domain_remove(subdomain)
-    if secondarydomain in domain_list()["domains"]:
-        domain_remove(secondarydomain)
+    userlist = user_list()["users"]
+    users = [ user for user in [ "alice", "bob" ] if user in userlist ]
+    users_remove(users)
 
+    domainlist = domain_list()["domains"]
+    domains = [ domain for domain in [ subdomain, secondarydomain ] if domain in domainlist ]
+    domains_remove(domains)
 
 def login(session, logged_as, logged_on=None):
 

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,62 @@ 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()
+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 +214,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 +332,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 +346,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 +356,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
@@ -302,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
@@ -314,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
@@ -327,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:
@@ -335,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"])