From 0361c3ac449f183476f7aee31439417d9f7f8012 Mon Sep 17 00:00:00 2001
From: Christian Poessinger <christian@poessinger.com>
Date: Sun, 13 Jun 2021 19:42:09 +0200
Subject: pppoe: T3621: validate that both username and password are set

A validator is missing checking that if authentication is used on a PPPoE
interface, both username and password are set.
---
 python/vyos/configverify.py                    | 13 +++++++++++++
 smoketest/scripts/cli/test_interfaces_pppoe.py | 14 ++++++++++++++
 src/conf_mode/interfaces-pppoe.py              |  2 ++
 3 files changed, 29 insertions(+)

diff --git a/python/vyos/configverify.py b/python/vyos/configverify.py
index 0ff45da2a..ee0fd94f7 100644
--- a/python/vyos/configverify.py
+++ b/python/vyos/configverify.py
@@ -122,6 +122,19 @@ def verify_mirror(config):
                 raise ConfigError(f'Can not mirror "{direction}" traffic back ' \
                                    'the originating interface!')
 
+def verify_authentication(config):
+    """
+    Common helper function used by interface implementations to perform
+    recurring validation of authentication for either PPPoE or WWAN interfaces.
+
+    If authentication CLI option is defined, both username and password must
+    be set!
+    """
+    if 'authentication' not in config:
+        return
+    if not {'user', 'password'} <= set(config['authentication']):
+        raise ConfigError('Authentication requires both username and ' \
+                          'password to be set!')
 
 def verify_address(config):
     """
diff --git a/smoketest/scripts/cli/test_interfaces_pppoe.py b/smoketest/scripts/cli/test_interfaces_pppoe.py
index 6bfe35d86..f7fa73ea2 100755
--- a/smoketest/scripts/cli/test_interfaces_pppoe.py
+++ b/smoketest/scripts/cli/test_interfaces_pppoe.py
@@ -158,5 +158,19 @@ class PPPoEInterfaceTest(unittest.TestCase):
             tmp = re.findall(f'systemctl restart dhcp6c@{interface}.service', tmp)
             self.assertTrue(tmp)
 
+    def test_pppoe_authentication(self):
+        # When username or password is set - so must be the other
+        interface = 'pppoe0'
+        self.session.set(base_path + [interface, 'authentication', 'user', 'vyos'])
+        self.session.set(base_path + [interface, 'source-interface', self._source_interface])
+        self.session.set(base_path + [interface, 'ipv6', 'address', 'autoconf'])
+
+        # check validate() - if user is set, so must be the password
+        with self.assertRaises(ConfigSessionError):
+            self.session.commit()
+
+        self.session.set(base_path + [interface, 'authentication', 'password', 'vyos'])
+        self.session.commit()
+
 if __name__ == '__main__':
     unittest.main(verbosity=2)
diff --git a/src/conf_mode/interfaces-pppoe.py b/src/conf_mode/interfaces-pppoe.py
index 3675db73b..6c4c6c95b 100755
--- a/src/conf_mode/interfaces-pppoe.py
+++ b/src/conf_mode/interfaces-pppoe.py
@@ -22,6 +22,7 @@ from netifaces import interfaces
 
 from vyos.config import Config
 from vyos.configdict import get_interface_dict
+from vyos.configverify import verify_authentication
 from vyos.configverify import verify_source_interface
 from vyos.configverify import verify_vrf
 from vyos.configverify import verify_mtu_ipv6
@@ -51,6 +52,7 @@ def verify(pppoe):
         return None
 
     verify_source_interface(pppoe)
+    verify_authentication(pppoe)
     verify_vrf(pppoe)
     verify_mtu_ipv6(pppoe)
 
-- 
cgit v1.2.3