Merge pull request #2002 from Zen3515/fix-cloudflare-ddns

T5219: ddclient: Allow not set login for Cloudflare API token

3 files changed, 18 insertions, 5 deletions

diff --git a/data/templates/dynamic-dns/ddclient.conf.j2 b/data/templates/dynamic-dns/ddclient.conf.j2
index c2c9b1dd6..e8ef5ac90 100644
--- a/data/templates/dynamic-dns/ddclient.conf.j2
+++ b/data/templates/dynamic-dns/ddclient.conf.j2
@@ -34,7 +34,9 @@ zone={{ config.zone }}
 # DynDNS provider configuration for {{ service }}, {{ dns_record }}
 protocol={{ config.protocol }},
 max-interval=28d,
+{%                     if config.login is vyos_defined %}
 login={{ config.login }},
+{%                     endif %}
 password='{{ config.password }}',
 {%                     if config.server is vyos_defined %}
 server={{ config.server }},
diff --git a/smoketest/scripts/cli/test_service_dns_dynamic.py b/smoketest/scripts/cli/test_service_dns_dynamic.py
index 57705e26f..a3aa41f94 100755
--- a/smoketest/scripts/cli/test_service_dns_dynamic.py
+++ b/smoketest/scripts/cli/test_service_dns_dynamic.py
@@ -45,22 +45,32 @@ class TestServiceDDNS(VyOSUnitTestSHIM.TestCase):
         self.cli_commit()
 
     def test_dyndns_service(self):
+        from itertools import product
         ddns = ['interface', interface, 'service']
+        users = [None, 'vyos_user']
         services = ['cloudflare', 'afraid', 'dyndns', 'zoneedit']
 
-        for service in services:
-            user = 'vyos_user'
+        for user, service in product(users, services):
             password = 'vyos_pass'
             zone = 'vyos.io'
             self.cli_delete(base_path)
             self.cli_set(base_path + ddns + [service, 'host-name', hostname])
-            self.cli_set(base_path + ddns + [service, 'login', user])
+            if user is not None:
+                self.cli_set(base_path + ddns + [service, 'login', user])
             self.cli_set(base_path + ddns + [service, 'password', password])
             self.cli_set(base_path + ddns + [service, 'zone', zone])
 
             # commit changes
             if service == 'cloudflare':
                 self.cli_commit()
+            elif user is None:
+                # not set user is only allowed for cloudflare
+                with self.assertRaises(ConfigSessionError):
+                    # remove zone to test not set user
+                    self.cli_delete(base_path + ddns + [service, 'zone', 'vyos.io'])
+                    self.cli_commit()
+                # this case is fininshed, user not set is not allowed when service isn't cloudflare
+                continue
             else:
                 # zone option only works on cloudflare, an exception is raised
                 # for all others
@@ -72,7 +82,7 @@ class TestServiceDDNS(VyOSUnitTestSHIM.TestCase):
 
             # we can only read the configuration file when we operate as 'root'
             protocol = get_config_value('protocol')
-            login = get_config_value('login')
+            login = None if user is None else get_config_value('login')
             pwd = get_config_value('password')
 
             # some services need special treatment
diff --git a/src/conf_mode/dynamic_dns.py b/src/conf_mode/dynamic_dns.py
index 06a2f7e15..426e3d693 100755
--- a/src/conf_mode/dynamic_dns.py
+++ b/src/conf_mode/dynamic_dns.py
@@ -108,7 +108,8 @@ def verify(dyndns):
                     raise ConfigError(f'"host-name" {error_msg}')
 
                 if 'login' not in config:
-                    raise ConfigError(f'"login" (username) {error_msg}')
+                    if service != 'cloudflare' and ('protocol' not in config or config['protocol'] != 'cloudflare'):
+                        raise ConfigError(f'"login" (username) {error_msg}, unless using CloudFlare')
 
                 if 'password' not in config:
                     raise ConfigError(f'"password" {error_msg}')