Merge branch 'dev' into dev

debian/changelog

@@ -1,3 +1,18 @@
+yunohost (11.1.5.5) stable; urgency=low
+
+  - admin->admins migration: try to handle boring case where the 'first' user cant be identified because it doesnt have the root@ alias (8485ebc7)
+  - appsv2: ignore the old/ugly/legacy removal of apt deps when removing the php conf, because that's handled by the apt resource (3bbba640)
+  - appsv2: moar fixes for v1->v2 upgrade not getting the proper env context (fb54da2e)
+
+ -- Alexandre Aubin <alex.aubin@mailoo.org>  Sat, 04 Feb 2023 18:51:03 +0100
+
+yunohost (11.1.5.4) stable; urgency=low
+
+  - appsv2: typo in ports resource doc x_x (0e787acb)
+  - appsv2: fix permission provisioning for fulldomain apps + fix apps not properly getting removed after failed resources init (476908bd)
+
+ -- Alexandre Aubin <alex.aubin@mailoo.org>  Fri, 03 Feb 2023 20:43:04 +0100
+
 yunohost (11.1.5.3) stable; urgency=low
 
   - helpers/appsv2: replacement of __PHPVERSION__ should use the phpversion setting, not YNH_PHP_VERSION (13d4e16e)

helpers/php

@@ -283,7 +283,7 @@ ynh_remove_fpm_config() {
     # If the PHP version used is not the default version for YunoHost
     # The second part with YNH_APP_PURGE is an ugly hack to guess that we're inside the remove script
     # (we don't actually care about its value, we just check its not empty hence it exists)
-    if [ "$phpversion" != "$YNH_DEFAULT_PHP_VERSION" ] && [ -n "${YNH_APP_PURGE:-}" ]; then
+    if [ "$phpversion" != "$YNH_DEFAULT_PHP_VERSION" ] && [ -n "${YNH_APP_PURGE:-}" ] && dpkg --compare-versions ${YNH_APP_PACKAGING_FORMAT:-0} lt 2; then
         # Remove app dependencies ... but ideally should happen via an explicit call from packager
         ynh_remove_app_dependencies
     fi

helpers/string

@@ -48,7 +48,7 @@ ynh_replace_string() {
     ynh_handle_getopts_args "$@"
     set +o xtrace # set +x
 
-    local delimit=@
+    local delimit=$'\001'
     # Escape the delimiter if it's in the string.
     match_string=${match_string//${delimit}/"\\${delimit}"}
     replace_string=${replace_string//${delimit}/"\\${delimit}"}

helpers/utils

@@ -568,7 +568,7 @@ ynh_read_var_in_file() {
     var_part+='\s*'
 
     # Extract the part after assignation sign
-    local expression_with_comment="$(tail +$line_number ${file} | grep -i -o -P $var_part'\K.*$' || echo YNH_NULL | head -n1)"
+    local expression_with_comment="$((tail +$line_number ${file} | grep -i -o -P $var_part'\K.*$' || echo YNH_NULL) | head -n1)"
     if [[ "$expression_with_comment" == "YNH_NULL" ]]; then
         set -o xtrace # set -x
         echo YNH_NULL
@@ -614,15 +614,14 @@ ynh_write_var_in_file() {
     set +o xtrace # set +x
 
     # Get the line number after which we search for the variable
-    local line_number=1
+    local after_line_number=1
     if [[ -n "$after" ]]; then
-        line_number=$(grep -m1 -n $after $file | cut -d: -f1)
-        if [[ -z "$line_number" ]]; then
+        after_line_number=$(grep -m1 -n $after $file | cut -d: -f1)
+        if [[ -z "$after_line_number" ]]; then
             set -o xtrace # set -x
             return 1
         fi
     fi
-    local range="${line_number},\$ "
 
     local filename="$(basename -- "$file")"
     local ext="${filename##*.}"
@@ -647,17 +646,21 @@ ynh_write_var_in_file() {
     var_part+='\s*'
 
     # Extract the part after assignation sign
-    local expression_with_comment="$(tail +$line_number ${file} | grep -i -o -P $var_part'\K.*$' || echo YNH_NULL | head -n1)"
+    local expression_with_comment="$((tail +$after_line_number ${file} | grep -i -o -P $var_part'\K.*$' || echo YNH_NULL) | head -n1)"
     if [[ "$expression_with_comment" == "YNH_NULL" ]]; then
         set -o xtrace # set -x
         return 1
     fi
+    local value_line_number="$(tail +$after_line_number ${file} | grep -m1 -n -i -P $var_part'\K.*$' | cut -d: -f1)"
+    value_line_number=$((after_line_number + value_line_number))
+    local range="${after_line_number},${value_line_number} "
 
     # Remove comments if needed
     local expression="$(echo "$expression_with_comment" | sed "s@${comments}[^$string]*\$@@g" | sed "s@\s*[$endline]*\s*]*\$@@")"
     endline=${expression_with_comment#"$expression"}
     endline="$(echo "$endline" | sed 's/\\/\\\\/g')"
     value="$(echo "$value" | sed 's/\\/\\\\/g')"
+    value=${value//&/"\&"}
     local first_char="${expression:0:1}"
     delimiter=$'\001'
     if [[ "$first_char" == '"' ]]; then

hooks/backup/18-data_multimedia

@@ -9,7 +9,7 @@ source /usr/share/yunohost/helpers
 # Backup destination
 backup_dir="${1}/data/multimedia"
 
-if [ -e "/home/yunohost.multimedia/.nobackup" ]; then
+if [ ! -e "/home/yunohost.multimedia" ] || [ -e "/home/yunohost.multimedia/.nobackup" ]; then
     exit 0
 fi
 

locales/en.json

@@ -345,9 +345,11 @@
     "domain_config_cert_summary_selfsigned": "WARNING: Current certificate is self-signed. Browsers will display a spooky warning to new visitors!",
     "domain_config_cert_validity": "Validity",
     "domain_config_default_app": "Default app",
+    "domain_config_default_app_help": "People will automatically be redirected to this app when opening this domain. If no app is specified, people are redirected to the user portal login form.",
     "domain_config_mail_in": "Incoming emails",
     "domain_config_mail_out": "Outgoing emails",
     "domain_config_xmpp": "Instant messaging (XMPP)",
+    "domain_config_xmpp_help": "NB: some XMPP features will require that you update your DNS records and regenerate your Lets Encrypt certificate to be enabled",
     "domain_created": "Domain created",
     "domain_creation_failed": "Unable to create domain {domain}: {error}",
     "domain_deleted": "Domain deleted",

share/config_domain.toml

@@ -9,8 +9,6 @@ name = "Features"
         type = "app"
         filter = "is_webapp"
         default = "_none"
-        # FIXME: i18n
-        help = "People will automatically be redirected to this app when opening this domain. If no app is specified, people are redirected to the user portal login form."
 
     [feature.mail]
 
@@ -27,8 +25,6 @@ name = "Features"
         [feature.xmpp.xmpp]
         type = "boolean"
         default = 0
-        # FIXME: i18n
-        help = "NB: some XMPP features will require that you update your DNS records and regenerate your Lets Encrypt certificate to be enabled"
 
 [dns]
 name = "DNS"
@@ -67,7 +63,6 @@ name = "Certificate"
         visible = "acme_eligible == false || acme_eligible == null"
 
         [cert.cert.cert_no_checks]
-        ask = "Ignore diagnosis checks"
         type = "boolean"
         default = false
         visible = "acme_eligible == false || acme_eligible == null"

src/app.py

@@ -1074,10 +1074,14 @@ def app_install(
     if packaging_format >= 2:
         from yunohost.utils.resources import AppResourceManager
 
-        AppResourceManager(app_instance_name, wanted=manifest, current={}).apply(
-            rollback_and_raise_exception_if_failure=True,
-            operation_logger=operation_logger,
-        )
+        try:
+            AppResourceManager(app_instance_name, wanted=manifest, current={}).apply(
+                rollback_and_raise_exception_if_failure=True,
+                operation_logger=operation_logger,
+            )
+        except (KeyboardInterrupt, EOFError, Exception) as e:
+            shutil.rmtree(app_setting_path)
+            raise e
     else:
         # Initialize the main permission for the app
         # The permission is initialized with no url associated, and with tile disabled
@@ -2651,22 +2655,31 @@ def _guess_webapp_path_requirement(app_folder: str) -> str:
     if len(domain_questions) == 1 and len(path_questions) == 1:
         return "domain_and_path"
     if len(domain_questions) == 1 and len(path_questions) == 0:
-        # This is likely to be a full-domain app...
 
-        # Confirm that this is a full-domain app This should cover most cases
-        # ...  though anyway the proper solution is to implement some mechanism
-        # in the manifest for app to declare that they require a full domain
-        # (among other thing) so that we can dynamically check/display this
-        # requirement on the webadmin form and not miserably fail at submit time
+        if manifest.get("packaging_format", 0) < 2:
 
-        # Full-domain apps typically declare something like path_url="/" or path=/
-        # and use ynh_webpath_register or yunohost_app_checkurl inside the install script
-        install_script_content = read_file(os.path.join(app_folder, "scripts/install"))
+            # This is likely to be a full-domain app...
 
-        if re.search(
-            r"\npath(_url)?=[\"']?/[\"']?", install_script_content
-        ) and re.search(r"ynh_webpath_register", install_script_content):
-            return "full_domain"
+            # Confirm that this is a full-domain app This should cover most cases
+            # ...  though anyway the proper solution is to implement some mechanism
+            # in the manifest for app to declare that they require a full domain
+            # (among other thing) so that we can dynamically check/display this
+            # requirement on the webadmin form and not miserably fail at submit time
+
+            # Full-domain apps typically declare something like path_url="/" or path=/
+            # and use ynh_webpath_register or yunohost_app_checkurl inside the install script
+            install_script_content = read_file(os.path.join(app_folder, "scripts/install"))
+
+            if re.search(
+                r"\npath(_url)?=[\"']?/[\"']?", install_script_content
+            ) and re.search(r"ynh_webpath_register", install_script_content):
+                return "full_domain"
+
+        else:
+            # For packaging v2 apps, check if there's a permission with url being a string
+            perm_resource = manifest.get("resources", {}).get("permissions")
+            if perm_resource is not None and isinstance(perm_resource.get("main", {}).get("url"), str):
+                return "full_domain"
 
     return "?"
 

src/backup.py

@@ -2295,7 +2295,7 @@ def backup_create(
     )
     backup_manager.backup()
 
-    logger.success(m18n.n("backup_created", name=name))
+    logger.success(m18n.n("backup_created", name=backup_manager.name))
     operation_logger.success()
 
     return {

src/dns.py

@@ -591,7 +591,10 @@ def _get_registrar_config_section(domain):
 
         # TODO : add a help tip with the link to the registar's API doc (c.f. Lexicon's README)
         registrar_list = read_toml(DOMAIN_REGISTRAR_LIST_PATH)
-        registrar_credentials = registrar_list[registrar]
+        registrar_credentials = registrar_list.get(registrar)
+        if registrar_credentials is None:
+            logger.warning(f"Registrar {registrar} unknown / Should be added to YunoHost's registrar_list.toml by the development team!")
+            registrar_credentials = {}
         for credential, infos in registrar_credentials.items():
             infos["default"] = infos.get("default", "")
             infos["optional"] = infos.get("optional", "False")

src/domain.py

@@ -624,14 +624,28 @@ class DomainConfigPanel(ConfigPanel):
                 f"domain_config_cert_summary_{status['summary']}"
             )
 
-            # Other specific strings used in config panels
-            # i18n: domain_config_cert_renew_help
-
             # FIXME: Ugly hack to save the cert status and reinject it in _load_current_values ...
             self.cert_status = status
 
         return toml
 
+    def get(self, key="", mode="classic"):
+        result = super().get(key=key, mode=mode)
+
+        if mode == "full":
+            for panel, section, option in self._iterate():
+                # This injects:
+                # i18n: domain_config_cert_renew_help
+                # i18n: domain_config_default_app_help
+                # i18n: domain_config_xmpp_help
+                if m18n.key_exists(self.config["i18n"] + "_" + option["id"] + "_help"):
+                    option["help"] = m18n.n(
+                        self.config["i18n"] + "_" + option["id"] + "_help"
+                    )
+            return self.config
+
+        return result
+
     def _load_current_values(self):
         # TODO add mechanism to share some settings with other domains on the same zone
         super()._load_current_values()

src/migrations/0026_new_admins_group.py

@@ -46,6 +46,19 @@ class MyMigration(Migration):
                 new_admin_user = user
                 break
 
+        # For some reason some system have no user with root@ alias,
+        # but the user does has admin / postmaster / ... alias
+        # ... try to find it instead otherwise this creashes the migration
+        # later because the admin@, postmaster@, .. aliases will already exist
+        if not new_admin_user:
+            for user in all_users:
+                aliases = user_info(user).get("mail-aliases", [])
+                if any(alias.startswith(f"admin@{main_domain}") for alias in aliases) \
+                  or any(alias.startswith(f"postmaster@{main_domain}") for alias in aliases):
+                    new_admin_user = user
+                    break
+
+
         self.ldap_migration_started = True
 
         if new_admin_user:

src/tests/test_backuprestore.py

@@ -6,6 +6,8 @@ from mock import patch
 
 from .conftest import message, raiseYunohostError, get_test_apps_dir
 
+from moulinette.utils.text import random_ascii
+
 from yunohost.app import app_install, app_remove, app_ssowatconf
 from yunohost.app import _is_installed
 from yunohost.backup import (
@@ -236,8 +238,9 @@ def add_archive_system_from_4p2():
 
 def test_backup_only_ldap(mocker):
     # Create the backup
-    with message(mocker, "backup_created"):
-        backup_create(system=["conf_ldap"], apps=None)
+    name = random_ascii(8)
+    with message(mocker, "backup_created", name=name):
+        backup_create(name=name, system=["conf_ldap"], apps=None)
 
     archives = backup_list()["archives"]
     assert len(archives) == 1
@@ -261,9 +264,10 @@ def test_backup_system_part_that_does_not_exists(mocker):
 
 
 def test_backup_and_restore_all_sys(mocker):
+    name = random_ascii(8)
     # Create the backup
-    with message(mocker, "backup_created"):
-        backup_create(system=[], apps=None)
+    with message(mocker, "backup_created", name=name):
+        backup_create(name=name, system=[], apps=None)
 
     archives = backup_list()["archives"]
     assert len(archives) == 1
@@ -294,9 +298,10 @@ def test_backup_and_restore_all_sys(mocker):
 
 @pytest.mark.with_system_archive_from_4p2
 def test_restore_system_from_Ynh4p2(monkeypatch, mocker):
+    name = random_ascii(8)
     # Backup current system
-    with message(mocker, "backup_created"):
-        backup_create(system=[], apps=None)
+    with message(mocker, "backup_created", name=name):
+        backup_create(name=name, system=[], apps=None)
     archives = backup_list()["archives"]
     assert len(archives) == 2
 
@@ -393,16 +398,17 @@ def test_backup_app_with_no_restore_script(mocker):
 
 @pytest.mark.clean_opt_dir
 def test_backup_with_different_output_directory(mocker):
+    name = random_ascii(8)
     # Create the backup
-    with message(mocker, "backup_created"):
+    with message(mocker, "backup_created", name=name):
         backup_create(
             system=["conf_ynh_settings"],
             apps=None,
             output_directory="/opt/test_backup_output_directory",
-            name="backup",
+            name=name,
         )
 
-    assert os.path.exists("/opt/test_backup_output_directory/backup.tar")
+    assert os.path.exists(f"/opt/test_backup_output_directory/{name}.tar")
 
     archives = backup_list()["archives"]
     assert len(archives) == 1
@@ -416,13 +422,14 @@ def test_backup_with_different_output_directory(mocker):
 @pytest.mark.clean_opt_dir
 def test_backup_using_copy_method(mocker):
     # Create the backup
-    with message(mocker, "backup_created"):
+    name = random_ascii(8)
+    with message(mocker, "backup_created", name=name):
         backup_create(
             system=["conf_ynh_settings"],
             apps=None,
             output_directory="/opt/test_backup_output_directory",
             methods=["copy"],
-            name="backup",
+            name=name,
         )
 
     assert os.path.exists("/opt/test_backup_output_directory/info.json")
@@ -565,8 +572,9 @@ def test_backup_and_restore_permission_app(mocker):
 
 def _test_backup_and_restore_app(mocker, app):
     # Create a backup of this app
-    with message(mocker, "backup_created"):
-        backup_create(system=None, apps=[app])
+    name = random_ascii(8)
+    with message(mocker, "backup_created", name=name):
+        backup_create(name=name, system=None, apps=[app])
 
     archives = backup_list()["archives"]
     assert len(archives) == 1
@@ -628,8 +636,9 @@ def test_restore_archive_with_custom_hook(mocker):
     os.system("touch %s/99-yolo" % custom_restore_hook_folder)
 
     # Backup with custom hook system
-    with message(mocker, "backup_created"):
-        backup_create(system=[], apps=None)
+    name = random_ascii(8)
+    with message(mocker, "backup_created", name=name):
+        backup_create(name=name, system=[], apps=None)
     archives = backup_list()["archives"]
     assert len(archives) == 1
 
@@ -666,5 +675,6 @@ def test_backup_binds_are_readonly(mocker, monkeypatch):
     )
 
     # Create the backup
-    with message(mocker, "backup_created"):
-        backup_create(system=[])
+    name = random_ascii(8)
+    with message(mocker, "backup_created", name=name):
+        backup_create(name=name, system=[])

src/utils/config.py

@@ -1359,7 +1359,7 @@ class GroupQuestion(Question):
 
         super().__init__(question, context)
 
-        self.choices = list(user_group_list(short=True)["groups"])
+        self.choices = list(user_group_list(short=True, include_primary_groups=False)["groups"])
 
         def _human_readable_group(g):
             # i18n: visitors

src/utils/resources.py

@@ -179,7 +179,7 @@ class AppResource:
         tmpdir = _make_tmp_workdir_for_app(app=self.app)
 
         env_ = _make_environment_for_app_script(
-            self.app, workdir=tmpdir, action=f"{action}_{self.type}"
+            self.app, workdir=tmpdir, action=f"{action}_{self.type}", include_app_settings=True,
         )
         env_.update(env)
 
@@ -320,6 +320,16 @@ class PermissionsResource(AppResource):
         # Delete legacy is_public setting if not already done
         self.delete_setting("is_public")
 
+        # Detect that we're using a full-domain app,
+        # in which case we probably need to automagically
+        # define the "path" setting with "/"
+        if (
+            isinstance(self.permissions["main"]["url"], str)
+            and self.get_setting("domain")
+            and not self.get_setting("path")
+        ):
+            self.set_setting("path", "/")
+
         existing_perms = user_permission_list(short=True, apps=[self.app])[
             "permissions"
         ]
@@ -338,6 +348,11 @@ class PermissionsResource(AppResource):
                     or self.get_setting(f"init_{perm}_permission")
                     or []
                 )
+
+                # If we're choosing 'visitors' from the init_{perm}_permission question, add all_users too
+                if not infos["allowed"] and init_allowed == "visitors":
+                    init_allowed = ["visitors", "all_users"]
+
                 permission_create(
                     perm_id,
                     allowed=init_allowed,
@@ -755,9 +770,10 @@ class PortsResource(AppResource):
 
     ##### Example:
     ```toml
-    [resources.port]
+    [resources.ports]
     # (empty should be fine for most apps... though you can customize stuff if absolutely needed)
 
+
     main.default = 12345    # if you really want to specify a prefered value .. but shouldnt matter in the majority of cases
 
     xmpp_client.default = 5222  # if you need another port, pick a name for it (here, "xmpp_client")