From 3e594ae63bb5300fa64a351208cedb612dfd0ed0 Mon Sep 17 00:00:00 2001 From: isabelmsft Date: Tue, 17 Jan 2023 06:34:09 +0000 Subject: [PATCH 1/5] add radius yang validation --- config/aaa.py | 51 ++++++++++++++++++++++++++++++++------------------- 1 file changed, 32 insertions(+), 19 deletions(-) diff --git a/config/aaa.py b/config/aaa.py index ffe974d4f6..2bcd0a83f4 100644 --- a/config/aaa.py +++ b/config/aaa.py @@ -498,15 +498,16 @@ def statistics(option): def add(address, retransmit, timeout, key, auth_type, auth_port, pri, use_mgmt_vrf, source_interface): """Specify a RADIUS server""" - if key: - if len(key) > RADIUS_PASSKEY_MAX_LEN: - click.echo('--key: Maximum of %d chars can be configured' % RADIUS_PASSKEY_MAX_LEN) - return - elif not is_secret(key): - click.echo('--key: ' + VALID_CHARS_MSG) - return + if ADHOC_VALIDATION: + if key: + if len(key) > RADIUS_PASSKEY_MAX_LEN: + click.echo('--key: Maximum of %d chars can be configured' % RADIUS_PASSKEY_MAX_LEN) + return + elif not is_secret(key): + click.echo('--key: ' + VALID_CHARS_MSG) + return - config_db = ConfigDBConnector() + config_db = ValidatedConfigDBConnector(ConfigDBConnector()) config_db.connect() old_data = config_db.get_table('RADIUS_SERVER') if address in old_data : @@ -529,16 +530,24 @@ def add(address, retransmit, timeout, key, auth_type, auth_port, pri, use_mgmt_v data['passkey'] = key if use_mgmt_vrf : data['vrf'] = "mgmt" - if source_interface : - if (source_interface.startswith("Ethernet") or \ - source_interface.startswith("PortChannel") or \ - source_interface.startswith("Vlan") or \ - source_interface.startswith("Loopback") or \ - source_interface == "eth0"): + if ADHOC_VALIDATION: + if source_interface : + if (source_interface.startswith("Ethernet") or \ + source_interface.startswith("PortChannel") or \ + source_interface.startswith("Vlan") or \ + source_interface.startswith("Loopback") or \ + source_interface == "eth0"): + data['src_intf'] = source_interface + else: + click.echo('Not supported interface name (valid interface name: Etherent/PortChannel/Vlan/Loopback/eth0)') + else: + if source_interface: data['src_intf'] = source_interface - else: - click.echo('Not supported interface name (valid interface name: Etherent/PortChannel/Vlan/Loopback/eth0)') - config_db.set_entry('RADIUS_SERVER', address, data) + try: + config_db.set_entry('RADIUS_SERVER', address, data) + except ValueError as e: + ctx = click.get_current_context() + ctx.fail("Invalid ConfigDB. Error: {}".format(e)) radius.add_command(add) @@ -549,7 +558,11 @@ def add(address, retransmit, timeout, key, auth_type, auth_port, pri, use_mgmt_v def delete(address): """Delete a RADIUS server""" - config_db = ConfigDBConnector() + config_db = ValidatedConfigDBConnector(ConfigDBConnector()) config_db.connect() - config_db.set_entry('RADIUS_SERVER', address, None) + try: + config_db.set_entry('RADIUS_SERVER', address, None) + except JsonPatchConflict as e: + ctx = click.get_current_context() + ctx.fail("Invalid ConfigDB. Error: {}".format(e)) radius.add_command(delete) From 7795b890a295fa10db087b957c6eecc1ee70ff3b Mon Sep 17 00:00:00 2001 From: isabelmsft Date: Tue, 17 Jan 2023 07:26:20 +0000 Subject: [PATCH 2/5] add radius yang validation --- config/aaa.py | 2 +- tests/radius_test.py | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/config/aaa.py b/config/aaa.py index 2bcd0a83f4..f5fdd5cc8a 100644 --- a/config/aaa.py +++ b/config/aaa.py @@ -562,7 +562,7 @@ def delete(address): config_db.connect() try: config_db.set_entry('RADIUS_SERVER', address, None) - except JsonPatchConflict as e: + except (JsonPointerException, JsonPatchConflict) as e: ctx = click.get_current_context() ctx.fail("Invalid ConfigDB. Error: {}".format(e)) radius.add_command(delete) diff --git a/tests/radius_test.py b/tests/radius_test.py index 117e19bde8..a676dd9de7 100644 --- a/tests/radius_test.py +++ b/tests/radius_test.py @@ -1,9 +1,11 @@ import imp import os import sys +import mock from click.testing import CliRunner from utilities_common.db import Db +from mock import patch import config.main as config import show.main as show @@ -71,6 +73,7 @@ def setup_class(cls): @classmethod def teardown_class(cls): os.environ['UTILITIES_UNIT_TESTING'] = "0" + config.ADHOC_VALIDATION = True print("TEARDOWN") def test_show_radius_default(self): @@ -192,3 +195,15 @@ def test_config_radius_authtype(self, get_cmd_module): assert result.exit_code == 0 assert result.output == show_radius_default_output + @patch("validated_config_db_connector.device_info.is_yang_config_validation_enabled", mock.Mock(return_value=True)) + @patch("config.validated_config_db_connector.ValidatedConfigDBConnector.validated_set_entry", mock.Mock(side_effect=ValueError)) + def test_config_radius_server_invalidkey_yang_validation(self): + config.ADHOC_VALIDATION = False + runner = CliRunner() + result = runner.invoke(config.config.commands["radius"],\ + ["add", "10.10.10.10", "-r", "1", "-t", "3",\ + "-k", "comma,invalid", "-s", "eth0"]) + print(result.output) + assert "Invalid ConfigDB. Error" in result.output + + From d55ae6af4c2d4228593ea64d682c6c87c64b5017 Mon Sep 17 00:00:00 2001 From: isabelmsft Date: Tue, 17 Jan 2023 07:27:45 +0000 Subject: [PATCH 3/5] fix format --- tests/radius_test.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/radius_test.py b/tests/radius_test.py index a676dd9de7..b82cedd259 100644 --- a/tests/radius_test.py +++ b/tests/radius_test.py @@ -73,7 +73,6 @@ def setup_class(cls): @classmethod def teardown_class(cls): os.environ['UTILITIES_UNIT_TESTING'] = "0" - config.ADHOC_VALIDATION = True print("TEARDOWN") def test_show_radius_default(self): @@ -205,5 +204,3 @@ def test_config_radius_server_invalidkey_yang_validation(self): "-k", "comma,invalid", "-s", "eth0"]) print(result.output) assert "Invalid ConfigDB. Error" in result.output - - From be140be10a9fc3d8fba9fdcbd33acd877d495190 Mon Sep 17 00:00:00 2001 From: isabelmsft Date: Tue, 17 Jan 2023 07:31:35 +0000 Subject: [PATCH 4/5] update UT --- tests/radius_test.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/radius_test.py b/tests/radius_test.py index b82cedd259..081d75fd6b 100644 --- a/tests/radius_test.py +++ b/tests/radius_test.py @@ -8,6 +8,7 @@ from mock import patch import config.main as config +import config.aaa as aaa import show.main as show test_path = os.path.dirname(os.path.abspath(__file__)) @@ -197,10 +198,12 @@ def test_config_radius_authtype(self, get_cmd_module): @patch("validated_config_db_connector.device_info.is_yang_config_validation_enabled", mock.Mock(return_value=True)) @patch("config.validated_config_db_connector.ValidatedConfigDBConnector.validated_set_entry", mock.Mock(side_effect=ValueError)) def test_config_radius_server_invalidkey_yang_validation(self): - config.ADHOC_VALIDATION = False + aaa.ADHOC_VALIDATION = False runner = CliRunner() result = runner.invoke(config.config.commands["radius"],\ ["add", "10.10.10.10", "-r", "1", "-t", "3",\ "-k", "comma,invalid", "-s", "eth0"]) print(result.output) assert "Invalid ConfigDB. Error" in result.output + + From 090cc350867b251faf375292096205d0a60ee29b Mon Sep 17 00:00:00 2001 From: isabelmsft Date: Tue, 17 Jan 2023 19:00:53 +0000 Subject: [PATCH 5/5] fix UT --- config/aaa.py | 1 + tests/radius_test.py | 12 +++++++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/config/aaa.py b/config/aaa.py index f5fdd5cc8a..6f4a42b340 100644 --- a/config/aaa.py +++ b/config/aaa.py @@ -4,6 +4,7 @@ from swsscommon.swsscommon import ConfigDBConnector from .validated_config_db_connector import ValidatedConfigDBConnector from jsonpatch import JsonPatchConflict +from jsonpointer import JsonPointerException import utilities_common.cli as clicommon ADHOC_VALIDATION = True diff --git a/tests/radius_test.py b/tests/radius_test.py index 081d75fd6b..49a1ac3ec4 100644 --- a/tests/radius_test.py +++ b/tests/radius_test.py @@ -2,10 +2,12 @@ import os import sys import mock +import jsonpatch from click.testing import CliRunner from utilities_common.db import Db from mock import patch +from jsonpointer import JsonPointerException import config.main as config import config.aaa as aaa @@ -206,4 +208,12 @@ def test_config_radius_server_invalidkey_yang_validation(self): print(result.output) assert "Invalid ConfigDB. Error" in result.output - + @patch("validated_config_db_connector.device_info.is_yang_config_validation_enabled", mock.Mock(return_value=True)) + @patch("config.validated_config_db_connector.ValidatedConfigDBConnector.validated_set_entry", mock.Mock(side_effect=JsonPointerException)) + def test_config_radius_server_invalid_delete_yang_validation(self): + aaa.ADHOC_VALIDATION = False + runner = CliRunner() + result = runner.invoke(config.config.commands["radius"],\ + ["delete", "10.10.10.x"]) + print(result.output) + assert "Invalid ConfigDB. Error" in result.output