From 1076f2124ea8cbcf5f9cf15f538c676575a41ee7 Mon Sep 17 00:00:00 2001 From: Michal Novak Date: Mon, 4 Sep 2023 15:42:48 +0200 Subject: [PATCH 1/3] implemented authentication, minor clean up Signed-off-by: Michal Novak --- src/confd_gnmi_api_adapter.py | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/src/confd_gnmi_api_adapter.py b/src/confd_gnmi_api_adapter.py index d69cbba..685930b 100644 --- a/src/confd_gnmi_api_adapter.py +++ b/src/confd_gnmi_api_adapter.py @@ -36,6 +36,13 @@ class GnmiConfDApiServerAdapter(GnmiServerAdapter): monitor_external_changes: bool = ApiAdapterDefaults.MONITOR_EXTERNAL_CHANGES external_port: int = ApiAdapterDefaults.EXTERNAL_PORT + class AuthenticationException(Exception): + def __init__(self, user, reason="N/A"): + self.user = user + self.reason = reason + def __str__(self): + return f"Authentication failed for user '{self.user}': {self.reason}." + def __init__(self): self.addr: str = GnmiConfDApiServerAdapter.addr self.port: int = GnmiConfDApiServerAdapter.port @@ -410,6 +417,17 @@ def set_credentials(self, username="admin", password="admin"): log.info("==> username=%s password=:-)", username) self.username = username self.password = password + auth = maapi.Maapi().authenticate(self.username, self.password, 1) + reason = "N/A" + if not isinstance(auth, int): + reason = auth[1] + auth = auth[0] + if auth == 1: + log.info(f"Authenticated {self.username}.") + else: + e = self.AuthenticationException(self.username, reason) + log.warning(e) + raise e log.info("<== self.username=%s self.password=:-)", self.username) # https://tools.ietf.org/html/rfc6022#page-8 @@ -538,7 +556,7 @@ def make_updates_with_maagic_rec(self, tr, node): elif isinstance(node, maagic.Leaf): yield from self.append_update(tr, node._path, node._cs_node) else: - if hasattr(node, "_children"): # skip nodes w/o children, e.g. Action + if hasattr(node, "_children"): # skip nodes w/o children, e.g. Action children = node._children.get_children(node._backend, node) if len(children) == 0 and isinstance(node, maagic.PresenceContainer): @@ -554,7 +572,6 @@ def make_updates_with_maagic(self, tr, path_str): else: yield from self.make_updates_with_maagic_rec(tr, node) - def get_updates(self, trans, path_str, confd_path, save_flags, allow_aggregation=False): log.debug("==> path_str=%s", path_str) tagpath = '/' + '/'.join(tag for tag, _ in parse_instance_path(path_str)) From e82983c8a45fc7be5115f1e1e09010050f933291 Mon Sep 17 00:00:00 2001 From: Michal Novak Date: Mon, 4 Sep 2023 16:33:51 +0200 Subject: [PATCH 2/3] authentication - renamed set_credentials to authenticate, review comments, cast of reason to str (can be sometimes int) Signed-off-by: Michal Novak --- src/confd_gnmi_api_adapter.py | 14 +++++++------- src/confd_gnmi_servicer.py | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/confd_gnmi_api_adapter.py b/src/confd_gnmi_api_adapter.py index 685930b..972984f 100644 --- a/src/confd_gnmi_api_adapter.py +++ b/src/confd_gnmi_api_adapter.py @@ -413,19 +413,19 @@ def get_subscription_handler(self, log.debug("<== handler=%s", handler) return handler - def set_credentials(self, username="admin", password="admin"): + def authenticate(self, username="admin", password="admin"): log.info("==> username=%s password=:-)", username) self.username = username self.password = password - auth = maapi.Maapi().authenticate(self.username, self.password, 1) + auth_status = maapi.Maapi().authenticate(self.username, self.password, 1) reason = "N/A" - if not isinstance(auth, int): - reason = auth[1] - auth = auth[0] - if auth == 1: + if not isinstance(auth_status, int): + reason = auth_status[1] + auth_status = auth_status[0] + if auth_status == 1: log.info(f"Authenticated {self.username}.") else: - e = self.AuthenticationException(self.username, reason) + e = self.AuthenticationException(self.username, str(reason)) log.warning(e) raise e log.info("<== self.username=%s self.password=:-)", self.username) diff --git a/src/confd_gnmi_servicer.py b/src/confd_gnmi_servicer.py index aac5744..406c749 100755 --- a/src/confd_gnmi_servicer.py +++ b/src/confd_gnmi_servicer.py @@ -42,7 +42,7 @@ def get_and_connect_adapter(self, username, password): elif self.adapter_type == AdapterType.API: from confd_gnmi_api_adapter import GnmiConfDApiServerAdapter adapter = GnmiConfDApiServerAdapter.get_adapter() - adapter.set_credentials(username=username, password=password) + adapter.authenticate(username=username, password=password) log.debug("<== adapter=%s", adapter) return adapter From 33650b903a4632115b188068bfeff88a9d5b0a0b Mon Sep 17 00:00:00 2001 From: Michal Novak Date: Tue, 5 Sep 2023 11:43:03 +0200 Subject: [PATCH 3/3] authentication - added tests Signed-off-by: Michal Novak --- tests/test_client_server_confd.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/test_client_server_confd.py b/tests/test_client_server_confd.py index 73bf6a1..9d8415b 100644 --- a/tests/test_client_server_confd.py +++ b/tests/test_client_server_confd.py @@ -6,10 +6,12 @@ import pytest +import grpc import gnmi_pb2 from client_server_test_base import GrpcBase from confd_gnmi_common import make_gnmi_path, make_xpath_path from confd_gnmi_server import AdapterType +from confd_gnmi_client import ConfDgNMIClient from route_status import RouteData, RouteProvider, ChangeOp from utils.utils import log sys.path.append(os.getenv('CONFD_DIR')+"/src/confd/pyapi/confd") @@ -168,3 +170,17 @@ def test_subscribe_stream_on_change_api_state(self, request): RouteProvider.stop_confd_loop() confd_thread.join() RouteProvider.close_dp() + + def _assert_auth(self, err_string, username="admin", password="admin"): + client = ConfDgNMIClient(username=username, password=password, insecure=True) + with pytest.raises(grpc.RpcError) as err: + capabilities = client.get_capabilities() + client.close() + assert err_string in str(err) + + @pytest.mark.confd + def test_authentication(self, request): + log.info("testing authentication") + self._assert_auth("Bad password", password="bad") + self._assert_auth("No such local user", username="bad", password="bad") + self._assert_auth("No such local user", username="bad")