dyndns: various tweaks to simplify the code, improve UX ...

locales/en.json

@@ -90,7 +90,8 @@
     "ask_new_domain": "New domain",
     "ask_new_path": "New path",
     "ask_password": "Password",
-    "ask_dyndns_recovery_password": "DynDNS recovey password",
+    "ask_dyndns_recovery_password_explain": "Please pick a recovery password for your DynDNS domain, in case you need to reset it later.",
+    "ask_dyndns_recovery_password": "DynDNS recovey passwory",
     "ask_user_domain": "Domain to use for the user's email address and XMPP account",
     "backup_abstract_method": "This backup method has yet to be implemented",
     "backup_actually_backuping": "Creating a backup archive from the collected files...",
@@ -383,8 +384,6 @@
     "domain_dns_registrar_supported": "YunoHost automatically detected that this domain is handled by the registrar **{registrar}**. If you want, YunoHost will automatically configure this DNS zone, if you provide it with the appropriate API credentials. You can find documentation on how to obtain your API credentials on this page: https://yunohost.org/registar_api_{registrar}. (You can also manually configure your DNS records following the documentation at https://yunohost.org/dns )",
     "domain_dns_registrar_yunohost": "This domain is a nohost.me / nohost.st / ynh.fr and its DNS configuration is therefore automatically handled by YunoHost without any further configuration. (see the 'yunohost dyndns update' command)",
     "domain_dyndns_already_subscribed": "You have already subscribed to a DynDNS domain",
-	"domain_dyndns_instruction_unclear": "You must choose exactly one of the following options : --subscribe or --ignore-dyndns",
-	"domain_dyndns_instruction_unclear_unsubscribe": "You must choose exactly one of the following options : --unsubscribe or --ignore-dyndns",
     "domain_exists": "The domain already exists",
     "domain_hostname_failed": "Unable to set new hostname. This might cause an issue later (it might be fine).",
     "domain_registrar_is_not_configured": "The registrar is not yet configured for domain {domain}.",
@@ -400,7 +399,6 @@
     "dyndns_domain_not_provided": "DynDNS provider {provider} cannot provide domain {domain}.",
     "dyndns_ip_update_failed": "Could not update IP address to DynDNS",
     "dyndns_ip_updated": "Updated your IP on DynDNS",
-    "dyndns_key_generating": "Generating DNS key... It may take a while.",
     "dyndns_key_not_found": "DNS key not found for the domain",
     "dyndns_no_domain_registered": "No domain registered with DynDNS",
 	"dyndns_no_recovery_password": "No recovery password specified! In case you loose control of this domain, you will need to contact an administrator in the YunoHost team!",

share/actionsmap.yml

@@ -493,7 +493,7 @@ domain:
                     help: Display domains as a tree
                     action: store_true
                 --features:
-                    help: List only domains with features enabled (xmpp, mail_in, mail_out, auto_push)
+                    help: List only domains with features enabled (xmpp, mail_in, mail_out)
                     nargs: "*"
 
         ### domain_info()

src/domain.py

@@ -228,7 +228,7 @@ def domain_add(operation_logger, domain, dyndns_recovery_password=None, ignore_d
     from yunohost.utils.password import assert_password_is_strong_enough
     from yunohost.certificate import _certificate_install_selfsigned
 
-    if dyndns_recovery_password != 0 and dyndns_recovery_password is not None:
+    if dyndns_recovery_password:
         operation_logger.data_to_redact.append(dyndns_recovery_password)
 
     if domain.startswith("xmpp-upload."):
@@ -252,35 +252,19 @@ def domain_add(operation_logger, domain, dyndns_recovery_password=None, ignore_d
     domain = domain.encode("idna").decode("utf-8")
 
     # Detect if this is a DynDNS domain ( and not a subdomain of a DynDNS domain )
-    dyndns = is_yunohost_dyndns_domain(domain) and len(domain.split(".")) == 3
+    dyndns = not ignore_dyndns and is_yunohost_dyndns_domain(domain) and len(domain.split(".")) == 3
     if dyndns:
-        if not ignore_dyndns and not dyndns_recovery_password:
-            if Moulinette.interface.type == "api":
-                raise YunohostValidationError("domain_dyndns_missing_password")
-            else:
-                dyndns_recovery_password = Moulinette.prompt(
-                    m18n.n("ask_dyndns_recovery_password"), is_password=True, confirm=True
-                )
-
-                # Ensure sufficiently complex password
-                assert_password_is_strong_enough("admin", dyndns_recovery_password)
-
-        if ((dyndns_recovery_password is None) == (ignore_dyndns is False)):
-            raise YunohostValidationError("domain_dyndns_instruction_unclear")
-
         from yunohost.dyndns import is_subscribing_allowed
-
         # Do not allow to subscribe to multiple dyndns domains...
         if not is_subscribing_allowed():
             raise YunohostValidationError("domain_dyndns_already_subscribed")
+        if dyndns_recovery_password:
+            assert_password_is_strong_enough("admin", dyndns_recovery_password)
 
     operation_logger.start()
-    if not dyndns and (dyndns_recovery_password is not None or ignore_dyndns):
-        logger.warning("This domain is not a DynDNS one, no need for the --dyndns-recovery-password or --ignore-dyndns option")
 
-    if dyndns and not ignore_dyndns:
+    if dyndns:
-        # Actually subscribe
+        domain_dyndns_subscribe(domain=domain, recovery_password=dyndns_recovery_password)
-        domain_dyndns_subscribe(domain=domain, password=dyndns_recovery_password)
 
     _certificate_install_selfsigned([domain], True)
 
@@ -346,7 +330,7 @@ def domain_remove(operation_logger, domain, remove_apps=False, force=False, dynd
     from yunohost.app import app_ssowatconf, app_info, app_remove
     from yunohost.utils.ldap import _get_ldap_interface
 
-    if dyndns_recovery_password != 0 and dyndns_recovery_password is not None:
+    if dyndns_recovery_password:
         operation_logger.data_to_redact.append(dyndns_recovery_password)
 
     # the 'force' here is related to the exception happening in domain_add ...
@@ -410,16 +394,10 @@ def domain_remove(operation_logger, domain, remove_apps=False, force=False, dynd
             )
 
     # Detect if this is a DynDNS domain ( and not a subdomain of a DynDNS domain )
-    dyndns = is_yunohost_dyndns_domain(domain) and len(domain.split(".")) == 3
+    dyndns = not ignore_dyndns and is_yunohost_dyndns_domain(domain) and len(domain.split(".")) == 3
-    if dyndns:
-        if ((dyndns_recovery_password is None) == (ignore_dyndns is False)):
-            raise YunohostValidationError("domain_dyndns_instruction_unclear_unsubscribe")
 
     operation_logger.start()
 
-    if not dyndns and ((dyndns_recovery_password is not None) or (ignore_dyndns is not False)):
-        logger.warning("This domain is not a DynDNS one, no need for the --dyndns_recovery_password or --ignore-dyndns option")
-
     ldap = _get_ldap_interface()
     try:
         ldap.remove("virtualdomain=" + domain + ",ou=domains")
@@ -465,9 +443,9 @@ def domain_remove(operation_logger, domain, remove_apps=False, force=False, dynd
     hook_callback("post_domain_remove", args=[domain])
 
     # If a password is provided, delete the DynDNS record
-    if dyndns and not ignore_dyndns:
+    if dyndns:
         # Actually unsubscribe
-        domain_dyndns_unsubscribe(domain=domain, password=dyndns_recovery_password)
+        domain_dyndns_unsubscribe(domain=domain, recovery_password=dyndns_recovery_password)
 
     logger.success(m18n.n("domain_deleted"))
 

src/dyndns.py

@@ -50,7 +50,7 @@ def is_subscribing_allowed():
     Returns:
         True if the limit is not reached, False otherwise
     """
-    return len(glob.glob("/etc/yunohost/dyndns/*.key")) < MAX_DYNDNS_DOMAINS
+    return len(dyndns_list()["domains"]) < MAX_DYNDNS_DOMAINS
 
 
 def _dyndns_available(domain):
@@ -78,7 +78,7 @@ def _dyndns_available(domain):
     return r == f"Domain {domain} is available"
 
 
-@is_unit_operation()
+@is_unit_operation(exclude=["recovery_password"])
 def dyndns_subscribe(operation_logger, domain=None, recovery_password=None):
     """
     Subscribe to a DynDNS service
@@ -88,26 +88,6 @@ def dyndns_subscribe(operation_logger, domain=None, recovery_password=None):
         recovery_password -- Password that will be used to delete the domain
     """
 
-    if recovery_password is None:
-        logger.warning(m18n.n('dyndns_no_recovery_password'))
-    else:
-        from yunohost.utils.password import assert_password_is_strong_enough
-        # Ensure sufficiently complex password
-        if Moulinette.interface.type == "cli" and recovery_password == 0:
-            recovery_password = Moulinette.prompt(
-                m18n.n("ask_password"),
-                is_password=True,
-                confirm=True
-            )
-        assert_password_is_strong_enough("admin", recovery_password)
-
-    if not is_subscribing_allowed():
-        raise YunohostValidationError("domain_dyndns_already_subscribed")
-
-    if domain is None:
-        domain = _get_maindomain()
-        operation_logger.related_to.append(("domain", domain))
-
     # Verify if domain is provided by subscribe_host
     if not is_yunohost_dyndns_domain(domain):
         raise YunohostValidationError(
@@ -118,6 +98,30 @@ def dyndns_subscribe(operation_logger, domain=None, recovery_password=None):
     if not _dyndns_available(domain):
         raise YunohostValidationError("dyndns_unavailable", domain=domain)
 
+    # Check adding another dyndns domain is still allowed
+    if not is_subscribing_allowed():
+        raise YunohostValidationError("domain_dyndns_already_subscribed")
+
+    # Prompt for a password if running in CLI and no password provided
+    if not recovery_password and Moulinette.interface.type == "cli":
+        logger.warning(m18n.n("ask_dyndns_recovery_password_explain"))
+        recovery_password = Moulinette.prompt(
+            m18n.n("ask_dyndns_recovery_password"),
+            is_password=True,
+            confirm=True
+        )
+    elif not recovery_password:
+        logger.warning(m18n.n("dyndns_no_recovery_password"))
+
+    if recovery_password:
+        from yunohost.utils.password import assert_password_is_strong_enough
+        assert_password_is_strong_enough("admin", recovery_password)
+        operation_logger.data_to_redact.append(recovery_password)
+
+    if domain is None:
+        domain = _get_maindomain()
+        operation_logger.related_to.append(("domain", domain))
+
     operation_logger.start()
 
     # '165' is the convention identifier for hmac-sha512 algorithm
@@ -127,8 +131,6 @@ def dyndns_subscribe(operation_logger, domain=None, recovery_password=None):
     if not os.path.exists("/etc/yunohost/dyndns"):
         os.makedirs("/etc/yunohost/dyndns")
 
-    logger.debug(m18n.n("dyndns_key_generating"))
-
     # Here, we emulate the behavior of the old 'dnssec-keygen' utility
     # which since bullseye was replaced by ddns-keygen which is now
     # in the bind9 package ... but installing bind9 will conflict with dnsmasq
@@ -154,7 +156,7 @@ def dyndns_subscribe(operation_logger, domain=None, recovery_password=None):
         # Yeah the secret is already a base64-encoded but we double-bas64-encode it, whatever...
         b64encoded_key = base64.b64encode(secret.encode()).decode()
         data = {"subdomain": domain}
-        if password is not None:
+        if recovery_password:
             data["recovery_password"] = hashlib.sha256((domain + ":" + recovery_password.strip()).encode('utf-8')).hexdigest()
         r = requests.post(
             f"https://{DYNDNS_PROVIDER}/key/{b64encoded_key}?key_algo=hmac-sha512",
@@ -188,7 +190,7 @@ def dyndns_subscribe(operation_logger, domain=None, recovery_password=None):
     logger.success(m18n.n("dyndns_registered"))
 
 
-@is_unit_operation()
+@is_unit_operation(exclude=["recovery_password"])
 def dyndns_unsubscribe(operation_logger, domain, recovery_password=None):
     """
     Unsubscribe from a DynDNS service
@@ -198,24 +200,19 @@ def dyndns_unsubscribe(operation_logger, domain, recovery_password=None):
         recovery_password -- Password that is used to delete the domain ( defined when subscribing )
     """
 
-    from yunohost.utils.password import assert_password_is_strong_enough
+    import requests  # lazy loading this module for performance reasons
+
+    # FIXME : it should be possible to unsubscribe the domain just using the key file ...
 
     # Ensure sufficiently complex password
     if Moulinette.interface.type == "cli" and not recovery_password:
         recovery_password = Moulinette.prompt(
-            m18n.n("ask_password"),
+            m18n.n("ask_dyndns_recovery_password"),
             is_password=True
         )
-    assert_password_is_strong_enough("admin", recovery_password)
 
     operation_logger.start()
 
-    # '165' is the convention identifier for hmac-sha512 algorithm
-    # '1234' is idk? doesnt matter, but the old format contained a number here...
-    key_file = f"/etc/yunohost/dyndns/K{domain}.+165+1234.key"
-
-    import requests  # lazy loading this module for performance reasons
-
     # Send delete request
     try:
         secret = str(domain) + ":" + str(recovery_password).strip()
@@ -228,30 +225,30 @@ def dyndns_unsubscribe(operation_logger, domain, recovery_password=None):
         raise YunohostError("dyndns_unregistration_failed", error=str(e))
 
     if r.status_code == 200:  # Deletion was successful
-        rm(key_file, force=True)
+        for key_file in glob.glob(f"/etc/yunohost/dyndns/K{domain}.+*.key"):
+            rm(key_file, force=True)
         # Yunohost regen conf will add the dyndns cron job if a key exists
         # in /etc/yunohost/dyndns
         regen_conf(["yunohost"])
-
-        logger.success(m18n.n("dyndns_unregistered"))
     elif r.status_code == 403:  # Wrong password
         raise YunohostError("dyndns_unsubscribe_wrong_password")
     elif r.status_code == 404:  # Invalid domain
         raise YunohostError("dyndns_unsubscribe_wrong_domain")
 
+    logger.success(m18n.n("dyndns_unregistered"))
+
 
 def dyndns_list():
     """
     Returns all currently subscribed DynDNS domains ( deduced from the key files )
     """
 
-    files = glob.glob("/etc/yunohost/dyndns/K*key")
+    from yunohost.domain import domain_list
-    # Get the domain names
-    for i in range(len(files)):
-        files[i] = files[i].split(".+", 1)[0]
-        files[i] = files[i].split("/etc/yunohost/dyndns/K")[1]
 
-    return {"domains": files}
+    domains = domain_list(exclude_subdomains=True)["domains"]
+    dyndns_domains = [d for d in domains if is_yunohost_dyndns_domain(d) and glob.glob(f"/etc/yunohost/dyndns/K{d}.+*.key")]
+
+    return {"domains": dyndns_domains}
 
 
 @is_unit_operation()
@@ -277,21 +274,14 @@ def dyndns_update(
     # If domain is not given, update all DynDNS domains
     if domain is None:
 
-        from yunohost.domain import domain_list
+        dyndns_domains = dyndns_list()["domains"]
 
-        domains = domain_list(exclude_subdomains=True, auto_push=True)["domains"]
+        if not dyndns_domains:
-        pushed = 0
-        for d in domains:
-            if is_yunohost_dyndns_domain(d):
-                dyndns_update(d, force=force, dry_run=dry_run)
-                pushed += 1
-        if pushed == 0:
             raise YunohostValidationError("dyndns_no_domain_registered")
-        return
 
-    elif type(domain).__name__ in ["list", "tuple"]:
+        for domain in dyndns_domains:
-        for d in domain:
+            dyndns_update(domain, force=force, dry_run=dry_run)
-            dyndns_update(d, force=force, dry_run=dry_run)
+
         return
 
     # If key is not given, pick the first file we find with the domain given

src/tools.py

@@ -197,11 +197,8 @@ def tools_postinstall(
     assert_password_is_strong_enough("admin", password)
 
     # If this is a nohost.me/noho.st, actually check for availability
-    if not ignore_dyndns and is_yunohost_dyndns_domain(domain):
+    dyndns = not ignore_dyndns and is_yunohost_dyndns_domain(domain)
-
+    if dyndns:
-        if (bool(dyndns_recovery_password), ignore_dyndns) in [(True, True), (False, False)]:
-            raise YunohostValidationError("domain_dyndns_instruction_unclear")
-
         # Check if the domain is available...
         try:
             available = _dyndns_available(domain)