From 724fad75cbdd83e68a727ce60746dcf1649f6683 Mon Sep 17 00:00:00 2001 From: samadpls Date: Sat, 24 Jun 2023 00:40:37 +0500 Subject: [PATCH 01/45] Implementws secure value loading method for multiple Windows credentials Signed-off-by: samadpls --- src/core/zowe/core_for_zowe_sdk/constants.py | 1 + .../zowe/core_for_zowe_sdk/zosmf_profile.py | 23 ++++++++++++++++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/core/zowe/core_for_zowe_sdk/constants.py b/src/core/zowe/core_for_zowe_sdk/constants.py index fa045b66..b56b3da8 100644 --- a/src/core/zowe/core_for_zowe_sdk/constants.py +++ b/src/core/zowe/core_for_zowe_sdk/constants.py @@ -16,4 +16,5 @@ "ZoweCredentialKey": "Zowe-Plugin", "ZoweServiceName": "Zowe", "ZoweAccountName": "secure_config_props", + "WIN32_CRED_MAX_STRING_LENGTH" : 2560 } diff --git a/src/core/zowe/core_for_zowe_sdk/zosmf_profile.py b/src/core/zowe/core_for_zowe_sdk/zosmf_profile.py index 0594ca10..e8a22b81 100644 --- a/src/core/zowe/core_for_zowe_sdk/zosmf_profile.py +++ b/src/core/zowe/core_for_zowe_sdk/zosmf_profile.py @@ -119,11 +119,32 @@ def __load_secure_credentials(self): try: zosmf_user = self.__get_secure_value("user") zosmf_password = self.__get_secure_value("password") + if sys.platform == "win32": + if len(zosmf_user) > constants["WIN32_CRED_MAX_STRING_LENGTH"]: + # Split user value across multiple fields + self.__split_and_store_secure_value("user", zosmf_user) + if len(zosmf_password) > constants["WIN32_CRED_MAX_STRING_LENGTH"]: + # Split password value across multiple fields + self.__split_and_store_secure_value("password", zosmf_password) except Exception as e: raise SecureProfileLoadFailed(self.profile_name, e) else: return (zosmf_user, zosmf_password) - + + def __split_and_store_secure_value(self, field_name, value): + """Split and store a secure value across multiple fields.""" + service_name = constants["ZoweCredentialKey"] + account_name = f"zosmf_{self.profile_name}_{field_name}" + + # Delete any previously used fields storing this value + keyring.delete_password(service_name, account_name) + sliced_values = [value[i:i+constants["WIN32_CRED_MAX_STRING_LENGTH"]] for i in range(0, len(value), constants["WIN32_CRED_MAX_STRING_LENGTH"])] + + for index, temp_value in enumerate(sliced_values, start=1): + keyring.set_password(service_name, f"{account_name}-{index}", temp_value) + + # Append null byte to mark the termination of the last field + keyring.set_password(service_name, f"{account_name}-{len(sliced_values)+1}", "\0") if HAS_KEYRING and sys.platform.startswith("linux"): from contextlib import closing From 47521068f239411217456c6f5d1096e1f1712acc Mon Sep 17 00:00:00 2001 From: samadpls Date: Tue, 27 Jun 2023 01:17:26 +0500 Subject: [PATCH 02/45] Modified implementation of the `__get_secure_value` method Signed-off-by: samadpls --- .../zowe/core_for_zowe_sdk/zosmf_profile.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/core/zowe/core_for_zowe_sdk/zosmf_profile.py b/src/core/zowe/core_for_zowe_sdk/zosmf_profile.py index e8a22b81..add68a12 100644 --- a/src/core/zowe/core_for_zowe_sdk/zosmf_profile.py +++ b/src/core/zowe/core_for_zowe_sdk/zosmf_profile.py @@ -94,13 +94,29 @@ def load(self): ) def __get_secure_value(self, name): + """Retrieve a secure value from the keyring.""" service_name = constants["ZoweCredentialKey"] account_name = "zosmf_{}_{}".format(self.profile_name, name) if sys.platform == "win32": service_name += "/" + account_name - secret_value = keyring.get_password(service_name, account_name) + secret_value = "" + + if sys.platform == "win32": + index = 1 + while True: + field_name = f"{account_name}-{index}" + temp_value = keyring.get_password(service_name, field_name) + if temp_value is None: + if index == 1: + # No fields with indexes, retrieve the value without an index + secret_value = keyring.get_password(service_name, account_name) + break + secret_value += temp_value + index += 1 + else: + secret_value = keyring.get_password(service_name, account_name) if sys.platform == "win32": secret_value = secret_value.encode("utf-16") From 6032bccbb7f012c38f73e2a76a6cca95f85f17b4 Mon Sep 17 00:00:00 2001 From: samadpls Date: Tue, 27 Jun 2023 02:39:53 +0500 Subject: [PATCH 03/45] fix condition statement Signed-off-by: samadpls --- src/core/zowe/core_for_zowe_sdk/zosmf_profile.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/core/zowe/core_for_zowe_sdk/zosmf_profile.py b/src/core/zowe/core_for_zowe_sdk/zosmf_profile.py index add68a12..04b3efd4 100644 --- a/src/core/zowe/core_for_zowe_sdk/zosmf_profile.py +++ b/src/core/zowe/core_for_zowe_sdk/zosmf_profile.py @@ -98,12 +98,9 @@ def __get_secure_value(self, name): service_name = constants["ZoweCredentialKey"] account_name = "zosmf_{}_{}".format(self.profile_name, name) - if sys.platform == "win32": - service_name += "/" + account_name - secret_value = "" - if sys.platform == "win32": + service_name += "/" + account_name index = 1 while True: field_name = f"{account_name}-{index}" From c5e997bbcdc00eb657e0a13778649434d4fcd596 Mon Sep 17 00:00:00 2001 From: samadpls Date: Tue, 27 Jun 2023 14:07:20 +0500 Subject: [PATCH 04/45] modified the logic Signed-off-by: samadpls --- .../zowe/core_for_zowe_sdk/zosmf_profile.py | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/core/zowe/core_for_zowe_sdk/zosmf_profile.py b/src/core/zowe/core_for_zowe_sdk/zosmf_profile.py index 04b3efd4..e5b47fe8 100644 --- a/src/core/zowe/core_for_zowe_sdk/zosmf_profile.py +++ b/src/core/zowe/core_for_zowe_sdk/zosmf_profile.py @@ -101,17 +101,17 @@ def __get_secure_value(self, name): secret_value = "" if sys.platform == "win32": service_name += "/" + account_name - index = 1 - while True: - field_name = f"{account_name}-{index}" - temp_value = keyring.get_password(service_name, field_name) - if temp_value is None: - if index == 1: - # No fields with indexes, retrieve the value without an index - secret_value = keyring.get_password(service_name, account_name) - break - secret_value += temp_value - index += 1 + + secret_value = keyring.get_password(service_name, account_name) + if secret_value is None: + index = 1 + while True: + field_name = f"{account_name}-{index}" + temp_value = keyring.get_password(service_name, field_name) + if temp_value is None: + break + secret_value += temp_value + index += 1 else: secret_value = keyring.get_password(service_name, account_name) From 078e8bb37c0fdf7c773cc9a6ef55965a99ffdd18 Mon Sep 17 00:00:00 2001 From: samadpls Date: Tue, 27 Jun 2023 14:17:00 +0500 Subject: [PATCH 05/45] added comments Signed-off-by: samadpls --- src/core/zowe/core_for_zowe_sdk/zosmf_profile.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/core/zowe/core_for_zowe_sdk/zosmf_profile.py b/src/core/zowe/core_for_zowe_sdk/zosmf_profile.py index e5b47fe8..4311d92b 100644 --- a/src/core/zowe/core_for_zowe_sdk/zosmf_profile.py +++ b/src/core/zowe/core_for_zowe_sdk/zosmf_profile.py @@ -102,8 +102,10 @@ def __get_secure_value(self, name): if sys.platform == "win32": service_name += "/" + account_name + # Try to retrieve the secure value without an index secret_value = keyring.get_password(service_name, account_name) if secret_value is None: + # Retrieve the secure value with an index index = 1 while True: field_name = f"{account_name}-{index}" From 94fff12718f3051d501f702c140d87910965b047 Mon Sep 17 00:00:00 2001 From: samadpls Date: Tue, 27 Jun 2023 20:41:49 +0500 Subject: [PATCH 06/45] still working not yet compelete Signed-off-by: samadpls --- .../zowe/core_for_zowe_sdk/zosmf_profile.py | 3 +- tests/unit/test_zowe_core.py | 70 ++++++++++++++++++- 2 files changed, 71 insertions(+), 2 deletions(-) diff --git a/src/core/zowe/core_for_zowe_sdk/zosmf_profile.py b/src/core/zowe/core_for_zowe_sdk/zosmf_profile.py index 4311d92b..0696dd31 100644 --- a/src/core/zowe/core_for_zowe_sdk/zosmf_profile.py +++ b/src/core/zowe/core_for_zowe_sdk/zosmf_profile.py @@ -72,7 +72,8 @@ def load(self): ) with open(profile_file, "r") as fileobj: - profile_yaml = yaml.safe_load(fileobj) + yaml_content = fileobj.read() + profile_yaml = yaml.safe_load(yaml_content) zosmf_host = profile_yaml["host"] if "port" in profile_yaml: diff --git a/tests/unit/test_zowe_core.py b/tests/unit/test_zowe_core.py index bdee79e0..6a6a2d98 100644 --- a/tests/unit/test_zowe_core.py +++ b/tests/unit/test_zowe_core.py @@ -12,7 +12,7 @@ from jsonschema import validate, ValidationError from zowe.core_for_zowe_sdk.validators import validate_config_json import commentjson - +from zowe.core_for_zowe_sdk.constants import constants from pyfakefs.fake_filesystem_unittest import TestCase from zowe.core_for_zowe_sdk import ( ApiConnection, @@ -158,6 +158,74 @@ def test_object_should_be_instance_of_class(self): zosmf_profile = ZosmfProfile(self.profile_name) self.assertIsInstance(zosmf_profile, ZosmfProfile) + @mock.patch("builtins.open", mock.mock_open(read_data="yaml_content")) + @mock.patch("yaml.safe_load") + def test_load(self, yaml_load_mock): + """Test the load method.""" + zosmf_profile = ZosmfProfile(self.profile_name) + profile_file = os.path.join( + zosmf_profile.profiles_dir, f"{self.profile_name}.yaml" + ) + + # Mock the return values + profile_yaml = { + "host": "example.com", + "port": 443, + "user": "test_user", + "password": "test_password", + "rejectUnauthorized": True, + } + yaml_load_mock.return_value = profile_yaml + # Set secure values for user and password + profile_yaml["user"] = constants["SecureValuePrefix"] + "secure_user" + profile_yaml["password"] = constants["SecureValuePrefix"] + "secure_password" + # Mock the private methods + # zosmf_profile._ZosmfProfile__load_secure_credentials = mock.MagicMock(return_value=("secure_user", "secure_password")) + zosmf_profile._ZosmfProfile__get_secure_value = mock.MagicMock(side_effect=["secure_user", "secure_password"]) + zosmf_profile._ZosmfProfile__split_and_store_secure_value = mock.MagicMock() + + # Call the load method + result = zosmf_profile.load() + + # Assertions + yaml_load_mock.assert_called_once_with("yaml_content") # Ensure safe_load is called with the correct argument + zosmf_profile._ZosmfProfile__load_secure_credentials.assert_called_once() + zosmf_profile._ZosmfProfile__get_secure_value.assert_called_with("user") + zosmf_profile._ZosmfProfile__get_secure_value.assert_called_with("password") + zosmf_profile._ZosmfProfile__split_and_store_secure_value.assert_not_called() + + expected_connection = ApiConnection( + "example.com", "secure_user", "secure_password", True + ) + self.assertEqual(result, expected_connection) + + # Additional test case for long credentials + long_user = "x" * 1000 + long_password = "y" * 2000 + profile_yaml["user"] = long_user + profile_yaml["password"] = long_password + + # Reset the mocks + yaml_load_mock.reset_mock() + zosmf_profile._ZosmfProfile__get_secure_value.reset_mock() + zosmf_profile._ZosmfProfile__split_and_store_secure_value.reset_mock() + + # Call the load method with long credentials + result = zosmf_profile.load() + + # Assertions + yaml_load_mock.assert_called_once_with("yaml_content") + zosmf_profile._ZosmfProfile__load_secure_credentials.assert_called_once() + zosmf_profile._ZosmfProfile__get_secure_value.assert_called_with("user") + zosmf_profile._ZosmfProfile__get_secure_value.assert_called_with("password") + zosmf_profile._ZosmfProfile__split_and_store_secure_value.assert_called_with("user", long_user) + zosmf_profile._ZosmfProfile__split_and_store_secure_value.assert_called_with("password", long_password) + + expected_connection = ApiConnection( + "example.com", "secure_user", "secure_password", True + ) + self.assertEqual(result, expected_connection) + class TestZosmfProfileManager(TestCase): """ProfileManager class unit tests.""" From eb24899cf5c7b6284b94f498c7d37cace978bbab Mon Sep 17 00:00:00 2001 From: samadpls Date: Wed, 28 Jun 2023 21:08:24 +0500 Subject: [PATCH 07/45] removed the logic from zosmf_profile Signed-off-by: samadpls --- .../zowe/core_for_zowe_sdk/zosmf_profile.py | 45 ++---------- tests/unit/test_zowe_core.py | 69 ------------------- 2 files changed, 4 insertions(+), 110 deletions(-) diff --git a/src/core/zowe/core_for_zowe_sdk/zosmf_profile.py b/src/core/zowe/core_for_zowe_sdk/zosmf_profile.py index 0696dd31..435630de 100644 --- a/src/core/zowe/core_for_zowe_sdk/zosmf_profile.py +++ b/src/core/zowe/core_for_zowe_sdk/zosmf_profile.py @@ -72,8 +72,7 @@ def load(self): ) with open(profile_file, "r") as fileobj: - yaml_content = fileobj.read() - profile_yaml = yaml.safe_load(yaml_content) + profile_yaml = yaml.safe_load(fileobj) zosmf_host = profile_yaml["host"] if "port" in profile_yaml: @@ -95,28 +94,13 @@ def load(self): ) def __get_secure_value(self, name): - """Retrieve a secure value from the keyring.""" service_name = constants["ZoweCredentialKey"] account_name = "zosmf_{}_{}".format(self.profile_name, name) - secret_value = "" if sys.platform == "win32": service_name += "/" + account_name - # Try to retrieve the secure value without an index - secret_value = keyring.get_password(service_name, account_name) - if secret_value is None: - # Retrieve the secure value with an index - index = 1 - while True: - field_name = f"{account_name}-{index}" - temp_value = keyring.get_password(service_name, field_name) - if temp_value is None: - break - secret_value += temp_value - index += 1 - else: - secret_value = keyring.get_password(service_name, account_name) + secret_value = keyring.get_password(service_name, account_name) if sys.platform == "win32": secret_value = secret_value.encode("utf-16") @@ -135,32 +119,11 @@ def __load_secure_credentials(self): try: zosmf_user = self.__get_secure_value("user") zosmf_password = self.__get_secure_value("password") - if sys.platform == "win32": - if len(zosmf_user) > constants["WIN32_CRED_MAX_STRING_LENGTH"]: - # Split user value across multiple fields - self.__split_and_store_secure_value("user", zosmf_user) - if len(zosmf_password) > constants["WIN32_CRED_MAX_STRING_LENGTH"]: - # Split password value across multiple fields - self.__split_and_store_secure_value("password", zosmf_password) except Exception as e: raise SecureProfileLoadFailed(self.profile_name, e) else: return (zosmf_user, zosmf_password) - - def __split_and_store_secure_value(self, field_name, value): - """Split and store a secure value across multiple fields.""" - service_name = constants["ZoweCredentialKey"] - account_name = f"zosmf_{self.profile_name}_{field_name}" - - # Delete any previously used fields storing this value - keyring.delete_password(service_name, account_name) - sliced_values = [value[i:i+constants["WIN32_CRED_MAX_STRING_LENGTH"]] for i in range(0, len(value), constants["WIN32_CRED_MAX_STRING_LENGTH"])] - - for index, temp_value in enumerate(sliced_values, start=1): - keyring.set_password(service_name, f"{account_name}-{index}", temp_value) - - # Append null byte to mark the termination of the last field - keyring.set_password(service_name, f"{account_name}-{len(sliced_values)+1}", "\0") + if HAS_KEYRING and sys.platform.startswith("linux"): from contextlib import closing @@ -193,4 +156,4 @@ def get_password(self, service, username): else: return self.__get_password(service, username, collection) - keyring.set_keyring(KeyringBackend()) + keyring.set_keyring(KeyringBackend()) \ No newline at end of file diff --git a/tests/unit/test_zowe_core.py b/tests/unit/test_zowe_core.py index 6a6a2d98..651954ba 100644 --- a/tests/unit/test_zowe_core.py +++ b/tests/unit/test_zowe_core.py @@ -158,75 +158,6 @@ def test_object_should_be_instance_of_class(self): zosmf_profile = ZosmfProfile(self.profile_name) self.assertIsInstance(zosmf_profile, ZosmfProfile) - @mock.patch("builtins.open", mock.mock_open(read_data="yaml_content")) - @mock.patch("yaml.safe_load") - def test_load(self, yaml_load_mock): - """Test the load method.""" - zosmf_profile = ZosmfProfile(self.profile_name) - profile_file = os.path.join( - zosmf_profile.profiles_dir, f"{self.profile_name}.yaml" - ) - - # Mock the return values - profile_yaml = { - "host": "example.com", - "port": 443, - "user": "test_user", - "password": "test_password", - "rejectUnauthorized": True, - } - yaml_load_mock.return_value = profile_yaml - # Set secure values for user and password - profile_yaml["user"] = constants["SecureValuePrefix"] + "secure_user" - profile_yaml["password"] = constants["SecureValuePrefix"] + "secure_password" - # Mock the private methods - # zosmf_profile._ZosmfProfile__load_secure_credentials = mock.MagicMock(return_value=("secure_user", "secure_password")) - zosmf_profile._ZosmfProfile__get_secure_value = mock.MagicMock(side_effect=["secure_user", "secure_password"]) - zosmf_profile._ZosmfProfile__split_and_store_secure_value = mock.MagicMock() - - # Call the load method - result = zosmf_profile.load() - - # Assertions - yaml_load_mock.assert_called_once_with("yaml_content") # Ensure safe_load is called with the correct argument - zosmf_profile._ZosmfProfile__load_secure_credentials.assert_called_once() - zosmf_profile._ZosmfProfile__get_secure_value.assert_called_with("user") - zosmf_profile._ZosmfProfile__get_secure_value.assert_called_with("password") - zosmf_profile._ZosmfProfile__split_and_store_secure_value.assert_not_called() - - expected_connection = ApiConnection( - "example.com", "secure_user", "secure_password", True - ) - self.assertEqual(result, expected_connection) - - # Additional test case for long credentials - long_user = "x" * 1000 - long_password = "y" * 2000 - profile_yaml["user"] = long_user - profile_yaml["password"] = long_password - - # Reset the mocks - yaml_load_mock.reset_mock() - zosmf_profile._ZosmfProfile__get_secure_value.reset_mock() - zosmf_profile._ZosmfProfile__split_and_store_secure_value.reset_mock() - - # Call the load method with long credentials - result = zosmf_profile.load() - - # Assertions - yaml_load_mock.assert_called_once_with("yaml_content") - zosmf_profile._ZosmfProfile__load_secure_credentials.assert_called_once() - zosmf_profile._ZosmfProfile__get_secure_value.assert_called_with("user") - zosmf_profile._ZosmfProfile__get_secure_value.assert_called_with("password") - zosmf_profile._ZosmfProfile__split_and_store_secure_value.assert_called_with("user", long_user) - zosmf_profile._ZosmfProfile__split_and_store_secure_value.assert_called_with("password", long_password) - - expected_connection = ApiConnection( - "example.com", "secure_user", "secure_password", True - ) - self.assertEqual(result, expected_connection) - - class TestZosmfProfileManager(TestCase): """ProfileManager class unit tests.""" From 079e298a403ff25cd56051e779f1a8aed8ee6e2d Mon Sep 17 00:00:00 2001 From: samadpls Date: Thu, 29 Jun 2023 17:23:06 +0500 Subject: [PATCH 08/45] implemneted the get and set logic in config_file Signed-off-by: samadpls --- .../zowe/core_for_zowe_sdk/config_file.py | 80 ++++++++++++++++++- 1 file changed, 76 insertions(+), 4 deletions(-) diff --git a/src/core/zowe/core_for_zowe_sdk/config_file.py b/src/core/zowe/core_for_zowe_sdk/config_file.py index 67eb33bf..57d9013c 100644 --- a/src/core/zowe/core_for_zowe_sdk/config_file.py +++ b/src/core/zowe/core_for_zowe_sdk/config_file.py @@ -312,10 +312,11 @@ def load_secure_props(self) -> None: if sys.platform == "win32": service_name += "/" + constants["ZoweAccountName"] - - secret_value = keyring.get_password( - service_name, constants["ZoweAccountName"] - ) + secret_value = self._retrieve_password(service_name) + else: + secret_value = keyring.get_password( + service_name, constants["ZoweAccountName"] + ) except Exception as exc: raise SecureProfileLoadFailed( @@ -340,3 +341,74 @@ def load_secure_props(self) -> None: f" with error '{error_msg}'", SecurePropsNotFoundWarning, ) + self.set_secure_props() + + def _retrieve_password(self, service_name: str) -> str: + """ + Retrieve the password from the keyring or storage. + If the password exceeds the maximum length, retrieve it in parts. + Parameters + ---------- + service_name: str + The service name for the password retrieval + Returns + ------- + str + The retrieved password + """ + password = keyring.get_password(service_name, constants["ZoweAccountName"]) + + if password is None: + # Retrieve the secure value with an index + index = 1 + while True: + field_name = f"{constants['ZoweAccountName']}-{index}" + temp_value = keyring.get_password(service_name, field_name) + if temp_value is None: + break + password += temp_value + index += 1 + + return password + + def set_secure_props(self) -> None: + """ + Set secure_props for the given config file + Returns + ------- + None + """ + if not HAS_KEYRING: + return + + try: + service_name = constants["ZoweServiceName"] + + if sys.platform == "win32": + service_name += "/" + constants["ZoweAccountName"] + credential = self.secure_props.get(self.filepath, "") + + if len(credential) > constants["WIN32_CRED_MAX_STRING_LENGTH"]: + # Split the credential string into chunks of maximum length + keyring.delete_password(service_name, constants["ZoweAccountName"]) + chunk_size = constants["WIN32_CRED_MAX_STRING_LENGTH"] + chunks = [credential[i : i + chunk_size] for i in range(0, len(credential), chunk_size)] + + # Set the individual chunks as separate keyring entries + for index, chunk in enumerate(chunks, start=1): + field_name = f"{constants['ZoweAccountName']}-{index}" + keyring.set_password(service_name, field_name, chunk) + else: + # Credential length is within the maximum limit, set it as a single keyring entry + keyring.set_password(service_name, constants["ZoweAccountName"], credential) + else: + credential = self.secure_props.get(self.filepath) + keyring.set_password(service_name, constants["ZoweAccountName"], credential) + + except KeyError as exc: + error_msg = str(exc) + warnings.warn( + f"No credentials found for loaded config file '{self.filepath}'" + f" with error '{error_msg}'", + SecurePropsNotFoundWarning, + ) From e5d817cbc02152393ed9188e2363ccb0941bd115 Mon Sep 17 00:00:00 2001 From: samadpls Date: Thu, 29 Jun 2023 17:23:56 +0500 Subject: [PATCH 09/45] typo Signed-off-by: samadpls --- src/core/zowe/core_for_zowe_sdk/config_file.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/core/zowe/core_for_zowe_sdk/config_file.py b/src/core/zowe/core_for_zowe_sdk/config_file.py index 57d9013c..120d1708 100644 --- a/src/core/zowe/core_for_zowe_sdk/config_file.py +++ b/src/core/zowe/core_for_zowe_sdk/config_file.py @@ -403,7 +403,9 @@ def set_secure_props(self) -> None: keyring.set_password(service_name, constants["ZoweAccountName"], credential) else: credential = self.secure_props.get(self.filepath) - keyring.set_password(service_name, constants["ZoweAccountName"], credential) + keyring.set_password( + service_name, constants["ZoweAccountName"], + credential) except KeyError as exc: error_msg = str(exc) From 1eeb12df1a33f49f484078257cfb906791c27542 Mon Sep 17 00:00:00 2001 From: samadpls Date: Thu, 29 Jun 2023 18:15:04 +0500 Subject: [PATCH 10/45] fixed typo Signed-off-by: samadpls --- src/core/zowe/core_for_zowe_sdk/config_file.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/zowe/core_for_zowe_sdk/config_file.py b/src/core/zowe/core_for_zowe_sdk/config_file.py index 120d1708..d591d811 100644 --- a/src/core/zowe/core_for_zowe_sdk/config_file.py +++ b/src/core/zowe/core_for_zowe_sdk/config_file.py @@ -341,7 +341,7 @@ def load_secure_props(self) -> None: f" with error '{error_msg}'", SecurePropsNotFoundWarning, ) - self.set_secure_props() + # self.set_secure_props() def _retrieve_password(self, service_name: str) -> str: """ From 1997c432e4bd6df299504e66b8c5b28b05c58aea Mon Sep 17 00:00:00 2001 From: samadpls Date: Thu, 29 Jun 2023 18:19:40 +0500 Subject: [PATCH 11/45] updated test file Signed-off-by: samadpls --- tests/unit/test_zowe_core.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/test_zowe_core.py b/tests/unit/test_zowe_core.py index 651954ba..595edc8f 100644 --- a/tests/unit/test_zowe_core.py +++ b/tests/unit/test_zowe_core.py @@ -12,7 +12,6 @@ from jsonschema import validate, ValidationError from zowe.core_for_zowe_sdk.validators import validate_config_json import commentjson -from zowe.core_for_zowe_sdk.constants import constants from pyfakefs.fake_filesystem_unittest import TestCase from zowe.core_for_zowe_sdk import ( ApiConnection, From d55cf994655da83d307085f08cad6c97ae4730cd Mon Sep 17 00:00:00 2001 From: samadpls Date: Mon, 3 Jul 2023 22:12:53 +0500 Subject: [PATCH 12/45] updated the logic Signed-off-by: samadpls --- src/core/zowe/core_for_zowe_sdk/config_file.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/core/zowe/core_for_zowe_sdk/config_file.py b/src/core/zowe/core_for_zowe_sdk/config_file.py index d591d811..9644c144 100644 --- a/src/core/zowe/core_for_zowe_sdk/config_file.py +++ b/src/core/zowe/core_for_zowe_sdk/config_file.py @@ -387,13 +387,22 @@ def set_secure_props(self) -> None: if sys.platform == "win32": service_name += "/" + constants["ZoweAccountName"] credential = self.secure_props.get(self.filepath, "") + # Load existing credentials + existing_credential = keyring.get_password(service_name, constants["ZoweAccountName"]) + + if existing_credential: + # Decode the existing credential and update secure_props + existing_secure_props = commentjson.loads(base64.b64decode(existing_credential).decode()) + existing_secure_props.update(self.secure_props.get(self.filepath, {})) + self.secure_props[self.filepath] = existing_secure_props if len(credential) > constants["WIN32_CRED_MAX_STRING_LENGTH"]: # Split the credential string into chunks of maximum length keyring.delete_password(service_name, constants["ZoweAccountName"]) chunk_size = constants["WIN32_CRED_MAX_STRING_LENGTH"] chunks = [credential[i : i + chunk_size] for i in range(0, len(credential), chunk_size)] - + # Append NUL byte to the last chunk + chunks[-1] += "\0" # Set the individual chunks as separate keyring entries for index, chunk in enumerate(chunks, start=1): field_name = f"{constants['ZoweAccountName']}-{index}" From b848160393f6741b1fbfe19f7dfad3285c7b7759 Mon Sep 17 00:00:00 2001 From: samadpls Date: Tue, 4 Jul 2023 19:11:41 +0500 Subject: [PATCH 13/45] updated the `_retrieve_password` and added a unit for it Signed-off-by: samadpls --- .../zowe/core_for_zowe_sdk/config_file.py | 17 ++++++------ tests/unit/test_zowe_core.py | 26 +++++++++++++++++++ 2 files changed, 35 insertions(+), 8 deletions(-) diff --git a/src/core/zowe/core_for_zowe_sdk/config_file.py b/src/core/zowe/core_for_zowe_sdk/config_file.py index 9644c144..5848f5ce 100644 --- a/src/core/zowe/core_for_zowe_sdk/config_file.py +++ b/src/core/zowe/core_for_zowe_sdk/config_file.py @@ -343,7 +343,7 @@ def load_secure_props(self) -> None: ) # self.set_secure_props() - def _retrieve_password(self, service_name: str) -> str: + def _retrieve_password(self, service_name: str) -> Optional[str]: """ Retrieve the password from the keyring or storage. If the password exceeds the maximum length, retrieve it in parts. @@ -361,15 +361,16 @@ def _retrieve_password(self, service_name: str) -> str: if password is None: # Retrieve the secure value with an index index = 1 - while True: - field_name = f"{constants['ZoweAccountName']}-{index}" - temp_value = keyring.get_password(service_name, field_name) - if temp_value is None: - break - password += temp_value + temp_value = keyring.get_password(service_name, f"{constants['ZoweAccountName']}-{index}") + while temp_value is not None: + if password is None: + password = temp_value + else: + password += temp_value index += 1 + temp_value = keyring.get_password(service_name, f"{constants['ZoweAccountName']}-{index}") - return password + return password def set_secure_props(self) -> None: """ diff --git a/tests/unit/test_zowe_core.py b/tests/unit/test_zowe_core.py index 595edc8f..5696ff88 100644 --- a/tests/unit/test_zowe_core.py +++ b/tests/unit/test_zowe_core.py @@ -23,6 +23,7 @@ exceptions, session_constants, custom_warnings, + constants, ) FIXTURES_PATH = os.path.join(os.path.dirname(os.path.abspath(__file__)), "fixtures") @@ -379,6 +380,31 @@ def test_profile_not_found_warning(self, get_pass_func): prof_manager.config_dir = self.custom_dir props: dict = prof_manager.load("non_existent_profile") + @patch("keyring.get_password", side_effect=["password", None, "part1","part2", None,None,None]) + def test_retrieve_password(self, get_pass_func): + # Set up mock values and expected results + # Create a ConfigFile instance and call the method being tested + service_name = "ZoweServiceName" + config_file = ConfigFile("User Config", "test") + retrieved_password = config_file._retrieve_password(service_name) + + # Scenario 1: Retrieve password directly + expected_password1 = "password" + self.assertEqual(retrieved_password, expected_password1) + get_pass_func.assert_called_with(service_name, constants["ZoweAccountName"]) + + # Scenario 2: Retrieve password in parts + retrieved_password = config_file._retrieve_password(service_name) + expected_password2 = "part1part2" + self.assertEqual(retrieved_password, expected_password2) + get_pass_func.assert_any_call(service_name, constants["ZoweAccountName"]) + get_pass_func.assert_any_call(service_name, f"{constants['ZoweAccountName']}-1") + get_pass_func.assert_any_call(service_name, f"{constants['ZoweAccountName']}-2") + + # Scenario 3: Password not found + retrieved_password = config_file._retrieve_password(service_name) + self.assertIsNone(retrieved_password) + get_pass_func.assert_called_with(service_name, f"{constants['ZoweAccountName']}-1") class TestValidateConfigJsonClass(unittest.TestCase): """Testing the validate_config_json function""" From 12caa4745232e8fb6c70310de8e2ad0025a113fa Mon Sep 17 00:00:00 2001 From: samadpls Date: Tue, 4 Jul 2023 20:48:27 +0500 Subject: [PATCH 14/45] updated `set_secure_props` and added unit test for testing `normal_credential` Signed-off-by: samadpls --- .../zowe/core_for_zowe_sdk/config_file.py | 2 +- tests/unit/test_zowe_core.py | 22 +++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/core/zowe/core_for_zowe_sdk/config_file.py b/src/core/zowe/core_for_zowe_sdk/config_file.py index 5848f5ce..a0cffdfe 100644 --- a/src/core/zowe/core_for_zowe_sdk/config_file.py +++ b/src/core/zowe/core_for_zowe_sdk/config_file.py @@ -393,7 +393,7 @@ def set_secure_props(self) -> None: if existing_credential: # Decode the existing credential and update secure_props - existing_secure_props = commentjson.loads(base64.b64decode(existing_credential).decode()) + existing_secure_props = commentjson.loads(existing_credential) existing_secure_props.update(self.secure_props.get(self.filepath, {})) self.secure_props[self.filepath] = existing_secure_props diff --git a/tests/unit/test_zowe_core.py b/tests/unit/test_zowe_core.py index 5696ff88..420e626e 100644 --- a/tests/unit/test_zowe_core.py +++ b/tests/unit/test_zowe_core.py @@ -406,6 +406,28 @@ def test_retrieve_password(self, get_pass_func): self.assertIsNone(retrieved_password) get_pass_func.assert_called_with(service_name, f"{constants['ZoweAccountName']}-1") + @mock.patch("keyring.get_password") + @mock.patch("keyring.set_password") + def test_set_secure_props_normal_credential(self, set_pass_func, get_pass_func): + # Set up mock values and expected results + service_name = constants["ZoweServiceName"] + credential = "test_credential" + existing_credential = '{"existing_key": "existing_value"}' + + # Mock the return values and behaviors of keyring functions + get_pass_func.return_value = existing_credential + # Create a ConfigFile instance and set secure_props + config_file = ConfigFile("User Config", "test") + config_file.secure_props = {"filepath": credential} + config_file.set_secure_props() + + # Verify the keyring function calls + set_pass_func.assert_called_once_with( + f"{service_name}/{constants['ZoweAccountName']}", + constants['ZoweAccountName'], + "" + ) + class TestValidateConfigJsonClass(unittest.TestCase): """Testing the validate_config_json function""" From 2bb7a1f59866f2b18b44b115aa7556ad719f519d Mon Sep 17 00:00:00 2001 From: samadpls Date: Tue, 4 Jul 2023 21:11:01 +0500 Subject: [PATCH 15/45] added Signed-off-by: samadpls --- tests/unit/test_zowe_core.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_zowe_core.py b/tests/unit/test_zowe_core.py index 420e626e..7d5b5c66 100644 --- a/tests/unit/test_zowe_core.py +++ b/tests/unit/test_zowe_core.py @@ -418,7 +418,7 @@ def test_set_secure_props_normal_credential(self, set_pass_func, get_pass_func): get_pass_func.return_value = existing_credential # Create a ConfigFile instance and set secure_props config_file = ConfigFile("User Config", "test") - config_file.secure_props = {"filepath": credential} + config_file.secure_props = {} config_file.set_secure_props() # Verify the keyring function calls From 0241a08b6a20f20294ded1ef771c1a1ce1548102 Mon Sep 17 00:00:00 2001 From: samadpls Date: Thu, 6 Jul 2023 00:23:13 +0500 Subject: [PATCH 16/45] Added unit test for `set_secure_props`, modified `_retrieve_password` & handled None case in `keyring.get_password` Signed-off-by: samadpls --- .../zowe/core_for_zowe_sdk/config_file.py | 52 ++++++++----- .../zowe/core_for_zowe_sdk/zosmf_profile.py | 4 + tests/unit/test_zowe_core.py | 77 +++++++++++++++++-- 3 files changed, 107 insertions(+), 26 deletions(-) diff --git a/src/core/zowe/core_for_zowe_sdk/config_file.py b/src/core/zowe/core_for_zowe_sdk/config_file.py index a0cffdfe..7846d5cb 100644 --- a/src/core/zowe/core_for_zowe_sdk/config_file.py +++ b/src/core/zowe/core_for_zowe_sdk/config_file.py @@ -317,6 +317,10 @@ def load_secure_props(self) -> None: secret_value = keyring.get_password( service_name, constants["ZoweAccountName"] ) + + # Handle the case when secret_value is None + if secret_value is None: + secret_value = "" except Exception as exc: raise SecureProfileLoadFailed( @@ -325,7 +329,7 @@ def load_secure_props(self) -> None: secure_config: str if sys.platform == "win32": - secure_config = secret_value.encode("utf-16") + secure_config = secret_value.encode("utf-16") else: secure_config = secret_value @@ -369,6 +373,9 @@ def _retrieve_password(self, service_name: str) -> Optional[str]: password += temp_value index += 1 temp_value = keyring.get_password(service_name, f"{constants['ZoweAccountName']}-{index}") + + if password is not None and password.endswith("\0"): + password = password[:-1] return password @@ -384,10 +391,10 @@ def set_secure_props(self) -> None: try: service_name = constants["ZoweServiceName"] - + credential = self.secure_props.get(self.filepath, {}) if sys.platform == "win32": service_name += "/" + constants["ZoweAccountName"] - credential = self.secure_props.get(self.filepath, "") + # Load existing credentials existing_credential = keyring.get_password(service_name, constants["ZoweAccountName"]) @@ -396,23 +403,32 @@ def set_secure_props(self) -> None: existing_secure_props = commentjson.loads(existing_credential) existing_secure_props.update(self.secure_props.get(self.filepath, {})) self.secure_props[self.filepath] = existing_secure_props - - if len(credential) > constants["WIN32_CRED_MAX_STRING_LENGTH"]: - # Split the credential string into chunks of maximum length - keyring.delete_password(service_name, constants["ZoweAccountName"]) - chunk_size = constants["WIN32_CRED_MAX_STRING_LENGTH"] - chunks = [credential[i : i + chunk_size] for i in range(0, len(credential), chunk_size)] - # Append NUL byte to the last chunk - chunks[-1] += "\0" - # Set the individual chunks as separate keyring entries - for index, chunk in enumerate(chunks, start=1): - field_name = f"{constants['ZoweAccountName']}-{index}" - keyring.set_password(service_name, field_name, chunk) + # Check if credential is a non-empty string + if credential: + # Get the username and password from the credential dictionary + username = credential.get("profiles.base.properties.user") + password = credential.get("profiles.base.properties.password") + + # Combine the username and password as "username:password" + username_password = f"{username}:{password}" + + # Encode the combined string as base64 + encoded_credential = base64.b64encode(username_password.encode()).decode() + if len(encoded_credential) > constants["WIN32_CRED_MAX_STRING_LENGTH"]: + # Split the encoded credential string into chunks of maximum length + keyring.delete_password(service_name, constants["ZoweAccountName"]) + chunk_size = constants["WIN32_CRED_MAX_STRING_LENGTH"] + chunks = [encoded_credential[i : i + chunk_size] for i in range(0, len(encoded_credential), chunk_size)] + # Append NUL byte to the last chunk + chunks[-1] += "\0" + # Set the individual chunks as separate keyring entries + for index, chunk in enumerate(chunks, start=1): + field_name = f"{constants['ZoweAccountName']}-{index}" + keyring.set_password(service_name, field_name, chunk) else: - # Credential length is within the maximum limit, set it as a single keyring entry - keyring.set_password(service_name, constants["ZoweAccountName"], credential) + # Credential length is within the maximum limit, set it as a single keyring entry + keyring.set_password(service_name, constants["ZoweAccountName"], "") else: - credential = self.secure_props.get(self.filepath) keyring.set_password( service_name, constants["ZoweAccountName"], credential) diff --git a/src/core/zowe/core_for_zowe_sdk/zosmf_profile.py b/src/core/zowe/core_for_zowe_sdk/zosmf_profile.py index 435630de..827b6ad8 100644 --- a/src/core/zowe/core_for_zowe_sdk/zosmf_profile.py +++ b/src/core/zowe/core_for_zowe_sdk/zosmf_profile.py @@ -101,6 +101,10 @@ def __get_secure_value(self, name): service_name += "/" + account_name secret_value = keyring.get_password(service_name, account_name) + + # Handle the case when secret_value is None + if secret_value is None: + secret_value = "" if sys.platform == "win32": secret_value = secret_value.encode("utf-16") diff --git a/tests/unit/test_zowe_core.py b/tests/unit/test_zowe_core.py index 7d5b5c66..ed1d036d 100644 --- a/tests/unit/test_zowe_core.py +++ b/tests/unit/test_zowe_core.py @@ -411,23 +411,84 @@ def test_retrieve_password(self, get_pass_func): def test_set_secure_props_normal_credential(self, set_pass_func, get_pass_func): # Set up mock values and expected results service_name = constants["ZoweServiceName"] - credential = "test_credential" - existing_credential = '{"existing_key": "existing_value"}' + # Setup - copy profile to fake filesystem created by pyfakefs + cwd_up_dir_path = os.path.dirname(CWD) + cwd_up_file_path = os.path.join(cwd_up_dir_path, "zowe.config.json") + os.chdir(CWD) + shutil.copy(self.original_file_path, cwd_up_file_path) + credential = { + "profiles.base.properties.user": "user", + "profiles.base.properties.password": "password" + } + self.setUpCreds(cwd_up_file_path,credential) + username = credential.get("profiles.base.properties.user") + password = credential.get("profiles.base.properties.password") + username_password = f"{username}:{password}" + encoded_credential = base64.b64encode( + username_password.encode() + ).decode() + + existing_credential = json.dumps(credential) - # Mock the return values and behaviors of keyring functions get_pass_func.return_value = existing_credential - # Create a ConfigFile instance and set secure_props - config_file = ConfigFile("User Config", "test") - config_file.secure_props = {} + + config_file = ConfigFile("User Config", "zowe.config.json",cwd_up_dir_path) + config_file.secure_props = {cwd_up_file_path: credential} config_file.set_secure_props() - # Verify the keyring function calls set_pass_func.assert_called_once_with( f"{service_name}/{constants['ZoweAccountName']}", constants['ZoweAccountName'], - "" + encoded_credential ) + @mock.patch("keyring.get_password") + @mock.patch("keyring.set_password") + @mock.patch("keyring.delete_password") + def test_set_secure_props_exceed_limit(self, delete_pass_func, set_pass_func, get_pass_func): + # Set up mock values and expected results + service_name = constants["ZoweServiceName"] + # Setup - copy profile to fake filesystem created by pyfakefs + cwd_up_dir_path = os.path.dirname(CWD) + cwd_up_file_path = os.path.join(cwd_up_dir_path, "zowe.config.json") + os.chdir(CWD) + shutil.copy(self.original_file_path, cwd_up_file_path) + credential = { + "profiles.base.properties.user": "user", + "profiles.base.properties.password": "a" * (constants["WIN32_CRED_MAX_STRING_LENGTH"] + 1) + } + self.setUpCreds(cwd_up_file_path, credential) + username = credential.get("profiles.base.properties.user") + password = credential.get("profiles.base.properties.password") + username_password = f"{username}:{password}" + encoded_credential = base64.b64encode(username_password.encode()).decode() + + existing_credential = json.dumps(credential) + + get_pass_func.return_value = existing_credential + + config_file = ConfigFile("User Config", "zowe.config.json", cwd_up_dir_path) + config_file.secure_props = {cwd_up_file_path: credential} + config_file.set_secure_props() + + # Verify the keyring function calls + delete_pass_func.assert_called_once_with( + f"{service_name}/{constants['ZoweAccountName']}", constants["ZoweAccountName"] + ) + + expected_calls = [] + chunk_size = constants["WIN32_CRED_MAX_STRING_LENGTH"] + chunks = [encoded_credential[i: i + chunk_size] for i in range(0, len(encoded_credential), chunk_size)] + chunks[-1] += "\0" + for index, chunk in enumerate(chunks, start=1): + field_name = f"{constants['ZoweAccountName']}-{index}" + expected_calls.append(mock.call( + f"{service_name}/{constants['ZoweAccountName']}", + field_name, + chunk + )) + set_pass_func.assert_has_calls(expected_calls) + class TestValidateConfigJsonClass(unittest.TestCase): """Testing the validate_config_json function""" From cb50b2647e28295ee3a346c3ec8ee710ea460d50 Mon Sep 17 00:00:00 2001 From: samadpls Date: Thu, 6 Jul 2023 19:23:48 +0500 Subject: [PATCH 17/45] updated the logic Signed-off-by: samadpls --- .../zowe/core_for_zowe_sdk/config_file.py | 74 ++++++++++--------- tests/unit/test_zowe_core.py | 2 +- 2 files changed, 39 insertions(+), 37 deletions(-) diff --git a/src/core/zowe/core_for_zowe_sdk/config_file.py b/src/core/zowe/core_for_zowe_sdk/config_file.py index 7846d5cb..b2224e9d 100644 --- a/src/core/zowe/core_for_zowe_sdk/config_file.py +++ b/src/core/zowe/core_for_zowe_sdk/config_file.py @@ -392,46 +392,48 @@ def set_secure_props(self) -> None: try: service_name = constants["ZoweServiceName"] credential = self.secure_props.get(self.filepath, {}) - if sys.platform == "win32": - service_name += "/" + constants["ZoweAccountName"] + # Check if credential is a non-empty string + if credential: + # Get the username and password from the credential dictionary + username = credential.get("profiles.base.properties.user") + password = credential.get("profiles.base.properties.password") - # Load existing credentials - existing_credential = keyring.get_password(service_name, constants["ZoweAccountName"]) - - if existing_credential: - # Decode the existing credential and update secure_props - existing_secure_props = commentjson.loads(existing_credential) - existing_secure_props.update(self.secure_props.get(self.filepath, {})) - self.secure_props[self.filepath] = existing_secure_props - # Check if credential is a non-empty string - if credential: - # Get the username and password from the credential dictionary - username = credential.get("profiles.base.properties.user") - password = credential.get("profiles.base.properties.password") + # Combine the username and password as "username:password" + username_password = f"{username}:{password}" + + # Encode the combined string as base64 + encoded_credential = base64.b64encode(username_password.encode()).decode() + print(encoded_credential) + if sys.platform == "win32": + service_name += "/" + constants["ZoweAccountName"] - # Combine the username and password as "username:password" - username_password = f"{username}:{password}" + # Load existing credentials + existing_credential = keyring.get_password(service_name, constants["ZoweAccountName"]) + print(existing_credential) + if existing_credential: + # Decode the existing credential and update secure_props + existing_secure_props = commentjson.loads(existing_credential) + existing_secure_props.update(self.secure_props.get(self.filepath, {})) + self.secure_props[self.filepath] = existing_secure_props - # Encode the combined string as base64 - encoded_credential = base64.b64encode(username_password.encode()).decode() - if len(encoded_credential) > constants["WIN32_CRED_MAX_STRING_LENGTH"]: - # Split the encoded credential string into chunks of maximum length - keyring.delete_password(service_name, constants["ZoweAccountName"]) - chunk_size = constants["WIN32_CRED_MAX_STRING_LENGTH"] - chunks = [encoded_credential[i : i + chunk_size] for i in range(0, len(encoded_credential), chunk_size)] - # Append NUL byte to the last chunk - chunks[-1] += "\0" - # Set the individual chunks as separate keyring entries - for index, chunk in enumerate(chunks, start=1): - field_name = f"{constants['ZoweAccountName']}-{index}" - keyring.set_password(service_name, field_name, chunk) + if len(encoded_credential) > constants["WIN32_CRED_MAX_STRING_LENGTH"]: + # Split the encoded credential string into chunks of maximum length + keyring.delete_password(service_name, constants["ZoweAccountName"]) + chunk_size = constants["WIN32_CRED_MAX_STRING_LENGTH"] + chunks = [encoded_credential[i : i + chunk_size] for i in range(0, len(encoded_credential), chunk_size)] + # Append NUL byte to the last chunk + chunks[-1] += "\0" + # Set the individual chunks as separate keyring entries + for index, chunk in enumerate(chunks, start=1): + field_name = f"{constants['ZoweAccountName']}-{index}" + keyring.set_password(service_name, field_name, chunk) + else: + # Credential length is within the maximum limit, set it as a single keyring entry + keyring.set_password(service_name, constants["ZoweAccountName"],encoded_credential) else: - # Credential length is within the maximum limit, set it as a single keyring entry - keyring.set_password(service_name, constants["ZoweAccountName"], "") - else: - keyring.set_password( - service_name, constants["ZoweAccountName"], - credential) + keyring.set_password( + service_name, constants["ZoweAccountName"], + encoded_credential) except KeyError as exc: error_msg = str(exc) diff --git a/tests/unit/test_zowe_core.py b/tests/unit/test_zowe_core.py index ed1d036d..afc474a8 100644 --- a/tests/unit/test_zowe_core.py +++ b/tests/unit/test_zowe_core.py @@ -380,7 +380,7 @@ def test_profile_not_found_warning(self, get_pass_func): prof_manager.config_dir = self.custom_dir props: dict = prof_manager.load("non_existent_profile") - @patch("keyring.get_password", side_effect=["password", None, "part1","part2", None,None,None]) + @patch("keyring.get_password", side_effect=["password", None, "part1","part2\0", None,None,None]) def test_retrieve_password(self, get_pass_func): # Set up mock values and expected results # Create a ConfigFile instance and call the method being tested From 4ac5b7df229638babcd7a125f44172a0a0f8af58 Mon Sep 17 00:00:00 2001 From: samadpls Date: Thu, 6 Jul 2023 20:16:15 +0500 Subject: [PATCH 18/45] updated testcases Signed-off-by: samadpls --- src/core/zowe/core_for_zowe_sdk/config_file.py | 2 -- tests/unit/test_zowe_core.py | 10 +++++----- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/core/zowe/core_for_zowe_sdk/config_file.py b/src/core/zowe/core_for_zowe_sdk/config_file.py index b2224e9d..649d6f85 100644 --- a/src/core/zowe/core_for_zowe_sdk/config_file.py +++ b/src/core/zowe/core_for_zowe_sdk/config_file.py @@ -403,13 +403,11 @@ def set_secure_props(self) -> None: # Encode the combined string as base64 encoded_credential = base64.b64encode(username_password.encode()).decode() - print(encoded_credential) if sys.platform == "win32": service_name += "/" + constants["ZoweAccountName"] # Load existing credentials existing_credential = keyring.get_password(service_name, constants["ZoweAccountName"]) - print(existing_credential) if existing_credential: # Decode the existing credential and update secure_props existing_secure_props = commentjson.loads(existing_credential) diff --git a/tests/unit/test_zowe_core.py b/tests/unit/test_zowe_core.py index afc474a8..888a5f84 100644 --- a/tests/unit/test_zowe_core.py +++ b/tests/unit/test_zowe_core.py @@ -410,7 +410,7 @@ def test_retrieve_password(self, get_pass_func): @mock.patch("keyring.set_password") def test_set_secure_props_normal_credential(self, set_pass_func, get_pass_func): # Set up mock values and expected results - service_name = constants["ZoweServiceName"] + service_name =constants["ZoweServiceName"] + "/" + constants["ZoweAccountName"] # Setup - copy profile to fake filesystem created by pyfakefs cwd_up_dir_path = os.path.dirname(CWD) cwd_up_file_path = os.path.join(cwd_up_dir_path, "zowe.config.json") @@ -437,7 +437,7 @@ def test_set_secure_props_normal_credential(self, set_pass_func, get_pass_func): config_file.set_secure_props() # Verify the keyring function calls set_pass_func.assert_called_once_with( - f"{service_name}/{constants['ZoweAccountName']}", + service_name, constants['ZoweAccountName'], encoded_credential ) @@ -447,7 +447,7 @@ def test_set_secure_props_normal_credential(self, set_pass_func, get_pass_func): @mock.patch("keyring.delete_password") def test_set_secure_props_exceed_limit(self, delete_pass_func, set_pass_func, get_pass_func): # Set up mock values and expected results - service_name = constants["ZoweServiceName"] + service_name =constants["ZoweServiceName"] + "/" + constants["ZoweAccountName"] # Setup - copy profile to fake filesystem created by pyfakefs cwd_up_dir_path = os.path.dirname(CWD) cwd_up_file_path = os.path.join(cwd_up_dir_path, "zowe.config.json") @@ -473,7 +473,7 @@ def test_set_secure_props_exceed_limit(self, delete_pass_func, set_pass_func, ge # Verify the keyring function calls delete_pass_func.assert_called_once_with( - f"{service_name}/{constants['ZoweAccountName']}", constants["ZoweAccountName"] + service_name, constants["ZoweAccountName"] ) expected_calls = [] @@ -483,7 +483,7 @@ def test_set_secure_props_exceed_limit(self, delete_pass_func, set_pass_func, ge for index, chunk in enumerate(chunks, start=1): field_name = f"{constants['ZoweAccountName']}-{index}" expected_calls.append(mock.call( - f"{service_name}/{constants['ZoweAccountName']}", + service_name, field_name, chunk )) From f75fc33d4b9d24ab5ef138f89ea8cb7edbe3007f Mon Sep 17 00:00:00 2001 From: samadpls Date: Thu, 6 Jul 2023 20:25:54 +0500 Subject: [PATCH 19/45] updated Signed-off-by: samadpls --- tests/unit/test_zowe_core.py | 146 ++++++++++++++++++----------------- 1 file changed, 74 insertions(+), 72 deletions(-) diff --git a/tests/unit/test_zowe_core.py b/tests/unit/test_zowe_core.py index 888a5f84..dfdb0764 100644 --- a/tests/unit/test_zowe_core.py +++ b/tests/unit/test_zowe_core.py @@ -410,84 +410,86 @@ def test_retrieve_password(self, get_pass_func): @mock.patch("keyring.set_password") def test_set_secure_props_normal_credential(self, set_pass_func, get_pass_func): # Set up mock values and expected results - service_name =constants["ZoweServiceName"] + "/" + constants["ZoweAccountName"] - # Setup - copy profile to fake filesystem created by pyfakefs - cwd_up_dir_path = os.path.dirname(CWD) - cwd_up_file_path = os.path.join(cwd_up_dir_path, "zowe.config.json") - os.chdir(CWD) - shutil.copy(self.original_file_path, cwd_up_file_path) - credential = { - "profiles.base.properties.user": "user", - "profiles.base.properties.password": "password" - } - self.setUpCreds(cwd_up_file_path,credential) - username = credential.get("profiles.base.properties.user") - password = credential.get("profiles.base.properties.password") - username_password = f"{username}:{password}" - encoded_credential = base64.b64encode( - username_password.encode() - ).decode() - - existing_credential = json.dumps(credential) - - get_pass_func.return_value = existing_credential - - config_file = ConfigFile("User Config", "zowe.config.json",cwd_up_dir_path) - config_file.secure_props = {cwd_up_file_path: credential} - config_file.set_secure_props() - # Verify the keyring function calls - set_pass_func.assert_called_once_with( - service_name, - constants['ZoweAccountName'], - encoded_credential - ) + if sys.platform == "win32": + service_name =constants["ZoweServiceName"] + "/" + constants["ZoweAccountName"] + # Setup - copy profile to fake filesystem created by pyfakefs + cwd_up_dir_path = os.path.dirname(CWD) + cwd_up_file_path = os.path.join(cwd_up_dir_path, "zowe.config.json") + os.chdir(CWD) + shutil.copy(self.original_file_path, cwd_up_file_path) + credential = { + "profiles.base.properties.user": "user", + "profiles.base.properties.password": "password" + } + self.setUpCreds(cwd_up_file_path,credential) + username = credential.get("profiles.base.properties.user") + password = credential.get("profiles.base.properties.password") + username_password = f"{username}:{password}" + encoded_credential = base64.b64encode( + username_password.encode() + ).decode() + + existing_credential = json.dumps(credential) + + get_pass_func.return_value = existing_credential + + config_file = ConfigFile("User Config", "zowe.config.json",cwd_up_dir_path) + config_file.secure_props = {cwd_up_file_path: credential} + config_file.set_secure_props() + # Verify the keyring function calls + set_pass_func.assert_called_once_with( + service_name, + constants['ZoweAccountName'], + encoded_credential + ) @mock.patch("keyring.get_password") @mock.patch("keyring.set_password") @mock.patch("keyring.delete_password") def test_set_secure_props_exceed_limit(self, delete_pass_func, set_pass_func, get_pass_func): - # Set up mock values and expected results - service_name =constants["ZoweServiceName"] + "/" + constants["ZoweAccountName"] - # Setup - copy profile to fake filesystem created by pyfakefs - cwd_up_dir_path = os.path.dirname(CWD) - cwd_up_file_path = os.path.join(cwd_up_dir_path, "zowe.config.json") - os.chdir(CWD) - shutil.copy(self.original_file_path, cwd_up_file_path) - credential = { - "profiles.base.properties.user": "user", - "profiles.base.properties.password": "a" * (constants["WIN32_CRED_MAX_STRING_LENGTH"] + 1) - } - self.setUpCreds(cwd_up_file_path, credential) - username = credential.get("profiles.base.properties.user") - password = credential.get("profiles.base.properties.password") - username_password = f"{username}:{password}" - encoded_credential = base64.b64encode(username_password.encode()).decode() - - existing_credential = json.dumps(credential) - - get_pass_func.return_value = existing_credential - - config_file = ConfigFile("User Config", "zowe.config.json", cwd_up_dir_path) - config_file.secure_props = {cwd_up_file_path: credential} - config_file.set_secure_props() - - # Verify the keyring function calls - delete_pass_func.assert_called_once_with( - service_name, constants["ZoweAccountName"] - ) + if sys.platform == "win32": + # Set up mock values and expected results + service_name =constants["ZoweServiceName"] + "/" + constants["ZoweAccountName"] + # Setup - copy profile to fake filesystem created by pyfakefs + cwd_up_dir_path = os.path.dirname(CWD) + cwd_up_file_path = os.path.join(cwd_up_dir_path, "zowe.config.json") + os.chdir(CWD) + shutil.copy(self.original_file_path, cwd_up_file_path) + credential = { + "profiles.base.properties.user": "user", + "profiles.base.properties.password": "a" * (constants["WIN32_CRED_MAX_STRING_LENGTH"] + 1) + } + self.setUpCreds(cwd_up_file_path, credential) + username = credential.get("profiles.base.properties.user") + password = credential.get("profiles.base.properties.password") + username_password = f"{username}:{password}" + encoded_credential = base64.b64encode(username_password.encode()).decode() + + existing_credential = json.dumps(credential) + + get_pass_func.return_value = existing_credential + + config_file = ConfigFile("User Config", "zowe.config.json", cwd_up_dir_path) + config_file.secure_props = {cwd_up_file_path: credential} + config_file.set_secure_props() + + # Verify the keyring function calls + delete_pass_func.assert_called_once_with( + service_name, constants["ZoweAccountName"] + ) - expected_calls = [] - chunk_size = constants["WIN32_CRED_MAX_STRING_LENGTH"] - chunks = [encoded_credential[i: i + chunk_size] for i in range(0, len(encoded_credential), chunk_size)] - chunks[-1] += "\0" - for index, chunk in enumerate(chunks, start=1): - field_name = f"{constants['ZoweAccountName']}-{index}" - expected_calls.append(mock.call( - service_name, - field_name, - chunk - )) - set_pass_func.assert_has_calls(expected_calls) + expected_calls = [] + chunk_size = constants["WIN32_CRED_MAX_STRING_LENGTH"] + chunks = [encoded_credential[i: i + chunk_size] for i in range(0, len(encoded_credential), chunk_size)] + chunks[-1] += "\0" + for index, chunk in enumerate(chunks, start=1): + field_name = f"{constants['ZoweAccountName']}-{index}" + expected_calls.append(mock.call( + service_name, + field_name, + chunk + )) + set_pass_func.assert_has_calls(expected_calls) class TestValidateConfigJsonClass(unittest.TestCase): """Testing the validate_config_json function""" From 1a4e21ef7dfd4db09c58b0fcb83b424c460d8c41 Mon Sep 17 00:00:00 2001 From: samadpls Date: Thu, 6 Jul 2023 20:47:45 +0500 Subject: [PATCH 20/45] updated a `CHANGELOG.md` Signed-off-by: samadpls --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c57aa5de..e513c5cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,3 +2,7 @@ All notable changes to the Zowe Client Python SDK will be documented in this file. +## Recent Changes + +- BugFix: Added a method to load secure values from multiple credential entries on Windows [#134](https://github.com/zowe/zowe-client-python-sdk/issues/134) + From 3c5e29343940cf1d1387cd1b1aac35befb5ccc15 Mon Sep 17 00:00:00 2001 From: samadpls Date: Thu, 6 Jul 2023 20:56:13 +0500 Subject: [PATCH 21/45] added mock patch Signed-off-by: samadpls --- tests/unit/test_zowe_core.py | 148 ++++++++++++++++++----------------- 1 file changed, 75 insertions(+), 73 deletions(-) diff --git a/tests/unit/test_zowe_core.py b/tests/unit/test_zowe_core.py index dfdb0764..43c47aad 100644 --- a/tests/unit/test_zowe_core.py +++ b/tests/unit/test_zowe_core.py @@ -406,90 +406,92 @@ def test_retrieve_password(self, get_pass_func): self.assertIsNone(retrieved_password) get_pass_func.assert_called_with(service_name, f"{constants['ZoweAccountName']}-1") + @mock.patch("sys.platform", "win32") @mock.patch("keyring.get_password") @mock.patch("keyring.set_password") def test_set_secure_props_normal_credential(self, set_pass_func, get_pass_func): # Set up mock values and expected results - if sys.platform == "win32": - service_name =constants["ZoweServiceName"] + "/" + constants["ZoweAccountName"] - # Setup - copy profile to fake filesystem created by pyfakefs - cwd_up_dir_path = os.path.dirname(CWD) - cwd_up_file_path = os.path.join(cwd_up_dir_path, "zowe.config.json") - os.chdir(CWD) - shutil.copy(self.original_file_path, cwd_up_file_path) - credential = { - "profiles.base.properties.user": "user", - "profiles.base.properties.password": "password" - } - self.setUpCreds(cwd_up_file_path,credential) - username = credential.get("profiles.base.properties.user") - password = credential.get("profiles.base.properties.password") - username_password = f"{username}:{password}" - encoded_credential = base64.b64encode( - username_password.encode() - ).decode() - - existing_credential = json.dumps(credential) - - get_pass_func.return_value = existing_credential - config_file = ConfigFile("User Config", "zowe.config.json",cwd_up_dir_path) - config_file.secure_props = {cwd_up_file_path: credential} - config_file.set_secure_props() - # Verify the keyring function calls - set_pass_func.assert_called_once_with( - service_name, - constants['ZoweAccountName'], - encoded_credential - ) + service_name =constants["ZoweServiceName"] + "/" + constants["ZoweAccountName"] + # Setup - copy profile to fake filesystem created by pyfakefs + cwd_up_dir_path = os.path.dirname(CWD) + cwd_up_file_path = os.path.join(cwd_up_dir_path, "zowe.config.json") + os.chdir(CWD) + shutil.copy(self.original_file_path, cwd_up_file_path) + credential = { + "profiles.base.properties.user": "user", + "profiles.base.properties.password": "password" + } + self.setUpCreds(cwd_up_file_path,credential) + username = credential.get("profiles.base.properties.user") + password = credential.get("profiles.base.properties.password") + username_password = f"{username}:{password}" + encoded_credential = base64.b64encode( + username_password.encode() + ).decode() + + existing_credential = json.dumps(credential) + + get_pass_func.return_value = existing_credential + + config_file = ConfigFile("User Config", "zowe.config.json",cwd_up_dir_path) + config_file.secure_props = {cwd_up_file_path: credential} + config_file.set_secure_props() + # Verify the keyring function calls + set_pass_func.assert_called_once_with( + service_name, + constants['ZoweAccountName'], + encoded_credential + ) + @mock.patch("sys.platform", "win32") @mock.patch("keyring.get_password") @mock.patch("keyring.set_password") @mock.patch("keyring.delete_password") def test_set_secure_props_exceed_limit(self, delete_pass_func, set_pass_func, get_pass_func): - if sys.platform == "win32": - # Set up mock values and expected results - service_name =constants["ZoweServiceName"] + "/" + constants["ZoweAccountName"] - # Setup - copy profile to fake filesystem created by pyfakefs - cwd_up_dir_path = os.path.dirname(CWD) - cwd_up_file_path = os.path.join(cwd_up_dir_path, "zowe.config.json") - os.chdir(CWD) - shutil.copy(self.original_file_path, cwd_up_file_path) - credential = { - "profiles.base.properties.user": "user", - "profiles.base.properties.password": "a" * (constants["WIN32_CRED_MAX_STRING_LENGTH"] + 1) - } - self.setUpCreds(cwd_up_file_path, credential) - username = credential.get("profiles.base.properties.user") - password = credential.get("profiles.base.properties.password") - username_password = f"{username}:{password}" - encoded_credential = base64.b64encode(username_password.encode()).decode() - - existing_credential = json.dumps(credential) - - get_pass_func.return_value = existing_credential - - config_file = ConfigFile("User Config", "zowe.config.json", cwd_up_dir_path) - config_file.secure_props = {cwd_up_file_path: credential} - config_file.set_secure_props() - - # Verify the keyring function calls - delete_pass_func.assert_called_once_with( - service_name, constants["ZoweAccountName"] - ) + + # Set up mock values and expected results + service_name =constants["ZoweServiceName"] + "/" + constants["ZoweAccountName"] + # Setup - copy profile to fake filesystem created by pyfakefs + cwd_up_dir_path = os.path.dirname(CWD) + cwd_up_file_path = os.path.join(cwd_up_dir_path, "zowe.config.json") + os.chdir(CWD) + shutil.copy(self.original_file_path, cwd_up_file_path) + credential = { + "profiles.base.properties.user": "user", + "profiles.base.properties.password": "a" * (constants["WIN32_CRED_MAX_STRING_LENGTH"] + 1) + } + self.setUpCreds(cwd_up_file_path, credential) + username = credential.get("profiles.base.properties.user") + password = credential.get("profiles.base.properties.password") + username_password = f"{username}:{password}" + encoded_credential = base64.b64encode(username_password.encode()).decode() + + existing_credential = json.dumps(credential) + + get_pass_func.return_value = existing_credential - expected_calls = [] - chunk_size = constants["WIN32_CRED_MAX_STRING_LENGTH"] - chunks = [encoded_credential[i: i + chunk_size] for i in range(0, len(encoded_credential), chunk_size)] - chunks[-1] += "\0" - for index, chunk in enumerate(chunks, start=1): - field_name = f"{constants['ZoweAccountName']}-{index}" - expected_calls.append(mock.call( - service_name, - field_name, - chunk - )) - set_pass_func.assert_has_calls(expected_calls) + config_file = ConfigFile("User Config", "zowe.config.json", cwd_up_dir_path) + config_file.secure_props = {cwd_up_file_path: credential} + config_file.set_secure_props() + + # Verify the keyring function calls + delete_pass_func.assert_called_once_with( + service_name, constants["ZoweAccountName"] + ) + + expected_calls = [] + chunk_size = constants["WIN32_CRED_MAX_STRING_LENGTH"] + chunks = [encoded_credential[i: i + chunk_size] for i in range(0, len(encoded_credential), chunk_size)] + chunks[-1] += "\0" + for index, chunk in enumerate(chunks, start=1): + field_name = f"{constants['ZoweAccountName']}-{index}" + expected_calls.append(mock.call( + service_name, + field_name, + chunk + )) + set_pass_func.assert_has_calls(expected_calls) class TestValidateConfigJsonClass(unittest.TestCase): """Testing the validate_config_json function""" From 7385860829d5699fdcc1ebefd26b6e1f2a312d25 Mon Sep 17 00:00:00 2001 From: samadpls Date: Wed, 19 Jul 2023 21:30:47 +0500 Subject: [PATCH 22/45] addressed comments and update test cases and `set_secure_prop` logic Signed-off-by: samadpls --- .../zowe/core_for_zowe_sdk/config_file.py | 58 +++++++-------- tests/unit/test_zowe_core.py | 72 +++++++++---------- 2 files changed, 61 insertions(+), 69 deletions(-) diff --git a/src/core/zowe/core_for_zowe_sdk/config_file.py b/src/core/zowe/core_for_zowe_sdk/config_file.py index 649d6f85..b2a04c97 100644 --- a/src/core/zowe/core_for_zowe_sdk/config_file.py +++ b/src/core/zowe/core_for_zowe_sdk/config_file.py @@ -329,7 +329,7 @@ def load_secure_props(self) -> None: secure_config: str if sys.platform == "win32": - secure_config = secret_value.encode("utf-16") + secure_config = secret_value.encode() else: secure_config = secret_value @@ -345,39 +345,39 @@ def load_secure_props(self) -> None: f" with error '{error_msg}'", SecurePropsNotFoundWarning, ) - # self.set_secure_props() - def _retrieve_password(self, service_name: str) -> Optional[str]: + def _retrieve_credential(self, service_name: str) -> Optional[str]: """ - Retrieve the password from the keyring or storage. - If the password exceeds the maximum length, retrieve it in parts. + Retrieve the credential from the keyring or storage. + If the credential exceeds the maximum length, retrieve it in parts. Parameters ---------- service_name: str - The service name for the password retrieval + The service name for the credential retrieval Returns ------- str - The retrieved password + The retrieved encoded credential """ - password = keyring.get_password(service_name, constants["ZoweAccountName"]) + encoded_credential = keyring.get_password(service_name, constants["ZoweAccountName"]) - if password is None: + if encoded_credential is None: # Retrieve the secure value with an index index = 1 - temp_value = keyring.get_password(service_name, f"{constants['ZoweAccountName']}-{index}") + temp_value = keyring.get_password(f"{service_name}-{index}", f"{constants['ZoweAccountName']}-{index}") while temp_value is not None: - if password is None: - password = temp_value + if encoded_credential is None: + encoded_credential = temp_value else: - password += temp_value + encoded_credential += temp_value index += 1 - temp_value = keyring.get_password(service_name, f"{constants['ZoweAccountName']}-{index}") + temp_value = keyring.get_password(f"{service_name}-{index}", f"{constants['ZoweAccountName']}-{index}") - if password is not None and password.endswith("\0"): - password = password[:-1] - - return password + if encoded_credential is not None and encoded_credential.endswith("\0"): + encoded_credential = encoded_credential[:-1] + + + return encoded_credential def set_secure_props(self) -> None: """ @@ -394,25 +394,21 @@ def set_secure_props(self) -> None: credential = self.secure_props.get(self.filepath, {}) # Check if credential is a non-empty string if credential: - # Get the username and password from the credential dictionary - username = credential.get("profiles.base.properties.user") - password = credential.get("profiles.base.properties.password") - # Combine the username and password as "username:password" - username_password = f"{username}:{password}" - - # Encode the combined string as base64 - encoded_credential = base64.b64encode(username_password.encode()).decode() + # Encode the credential + encoded_credential = base64.b64encode(commentjson.dumps(credential).encode()).decode() if sys.platform == "win32": service_name += "/" + constants["ZoweAccountName"] # Load existing credentials - existing_credential = keyring.get_password(service_name, constants["ZoweAccountName"]) + existing_credential = self._retrieve_credential(service_name) if existing_credential: # Decode the existing credential and update secure_props - existing_secure_props = commentjson.loads(existing_credential) - existing_secure_props.update(self.secure_props.get(self.filepath, {})) - self.secure_props[self.filepath] = existing_secure_props + existing_credential_bytes = base64.b64decode(existing_credential.encode()) + existing_secure_props = commentjson.loads(existing_credential_bytes) + existing_secure_props[self.filepath] = self.secure_props[self.filepath] + encoded_credential = base64.b64encode(commentjson.dumps(existing_secure_props).encode()).decode() + if len(encoded_credential) > constants["WIN32_CRED_MAX_STRING_LENGTH"]: # Split the encoded credential string into chunks of maximum length @@ -424,7 +420,7 @@ def set_secure_props(self) -> None: # Set the individual chunks as separate keyring entries for index, chunk in enumerate(chunks, start=1): field_name = f"{constants['ZoweAccountName']}-{index}" - keyring.set_password(service_name, field_name, chunk) + keyring.set_password(f"{service_name}-{index}", field_name, chunk) else: # Credential length is within the maximum limit, set it as a single keyring entry keyring.set_password(service_name, constants["ZoweAccountName"],encoded_credential) diff --git a/tests/unit/test_zowe_core.py b/tests/unit/test_zowe_core.py index 43c47aad..0866e7ce 100644 --- a/tests/unit/test_zowe_core.py +++ b/tests/unit/test_zowe_core.py @@ -381,35 +381,35 @@ def test_profile_not_found_warning(self, get_pass_func): props: dict = prof_manager.load("non_existent_profile") @patch("keyring.get_password", side_effect=["password", None, "part1","part2\0", None,None,None]) - def test_retrieve_password(self, get_pass_func): + def test_retrieve_credential(self, get_pass_func): # Set up mock values and expected results # Create a ConfigFile instance and call the method being tested service_name = "ZoweServiceName" config_file = ConfigFile("User Config", "test") - retrieved_password = config_file._retrieve_password(service_name) + retrieve_credential = config_file._retrieve_credential(service_name) # Scenario 1: Retrieve password directly expected_password1 = "password" - self.assertEqual(retrieved_password, expected_password1) + self.assertEqual(retrieve_credential, expected_password1) get_pass_func.assert_called_with(service_name, constants["ZoweAccountName"]) # Scenario 2: Retrieve password in parts - retrieved_password = config_file._retrieve_password(service_name) + retrieve_credential = config_file._retrieve_credential(service_name) expected_password2 = "part1part2" - self.assertEqual(retrieved_password, expected_password2) + self.assertEqual(retrieve_credential, expected_password2) get_pass_func.assert_any_call(service_name, constants["ZoweAccountName"]) - get_pass_func.assert_any_call(service_name, f"{constants['ZoweAccountName']}-1") - get_pass_func.assert_any_call(service_name, f"{constants['ZoweAccountName']}-2") + get_pass_func.assert_any_call(f"{service_name}-1", f"{constants['ZoweAccountName']}-1") + get_pass_func.assert_any_call(f"{service_name}-2", f"{constants['ZoweAccountName']}-2") # Scenario 3: Password not found - retrieved_password = config_file._retrieve_password(service_name) - self.assertIsNone(retrieved_password) - get_pass_func.assert_called_with(service_name, f"{constants['ZoweAccountName']}-1") + retrieve_credential = config_file._retrieve_credential(service_name) + self.assertIsNone(retrieve_credential) + get_pass_func.assert_called_with(f"{service_name}-1", f"{constants['ZoweAccountName']}-1") @mock.patch("sys.platform", "win32") - @mock.patch("keyring.get_password") @mock.patch("keyring.set_password") - def test_set_secure_props_normal_credential(self, set_pass_func, get_pass_func): + @mock.patch("zowe.core_for_zowe_sdk.ConfigFile._retrieve_credential") + def test_set_secure_props_normal_credential(self, retrieve_cred_func, set_pass_func): # Set up mock values and expected results service_name =constants["ZoweServiceName"] + "/" + constants["ZoweAccountName"] @@ -419,25 +419,23 @@ def test_set_secure_props_normal_credential(self, set_pass_func, get_pass_func): os.chdir(CWD) shutil.copy(self.original_file_path, cwd_up_file_path) credential = { + cwd_up_file_path: + { "profiles.base.properties.user": "user", "profiles.base.properties.password": "password" + } } self.setUpCreds(cwd_up_file_path,credential) - username = credential.get("profiles.base.properties.user") - password = credential.get("profiles.base.properties.password") - username_password = f"{username}:{password}" - encoded_credential = base64.b64encode( - username_password.encode() - ).decode() - - existing_credential = json.dumps(credential) - - get_pass_func.return_value = existing_credential - - config_file = ConfigFile("User Config", "zowe.config.json",cwd_up_dir_path) - config_file.secure_props = {cwd_up_file_path: credential} + + encoded_credential = base64.b64encode(commentjson.dumps(credential).encode()).decode() + + retrieve_cred_func.return_value = encoded_credential + + + config_file = ConfigFile("User Config", "zowe.config.json", cwd_up_dir_path) + config_file.secure_props = credential config_file.set_secure_props() - # Verify the keyring function calls + # Verify the keyring function call set_pass_func.assert_called_once_with( service_name, constants['ZoweAccountName'], @@ -445,10 +443,10 @@ def test_set_secure_props_normal_credential(self, set_pass_func, get_pass_func): ) @mock.patch("sys.platform", "win32") - @mock.patch("keyring.get_password") + @mock.patch("zowe.core_for_zowe_sdk.ConfigFile._retrieve_credential") @mock.patch("keyring.set_password") @mock.patch("keyring.delete_password") - def test_set_secure_props_exceed_limit(self, delete_pass_func, set_pass_func, get_pass_func): + def test_set_secure_props_exceed_limit(self, delete_pass_func, set_pass_func, retrieve_cred_func): # Set up mock values and expected results service_name =constants["ZoweServiceName"] + "/" + constants["ZoweAccountName"] @@ -458,21 +456,18 @@ def test_set_secure_props_exceed_limit(self, delete_pass_func, set_pass_func, ge os.chdir(CWD) shutil.copy(self.original_file_path, cwd_up_file_path) credential = { + cwd_up_file_path: + { "profiles.base.properties.user": "user", "profiles.base.properties.password": "a" * (constants["WIN32_CRED_MAX_STRING_LENGTH"] + 1) + } } self.setUpCreds(cwd_up_file_path, credential) - username = credential.get("profiles.base.properties.user") - password = credential.get("profiles.base.properties.password") - username_password = f"{username}:{password}" - encoded_credential = base64.b64encode(username_password.encode()).decode() - - existing_credential = json.dumps(credential) - - get_pass_func.return_value = existing_credential + encoded_credential = base64.b64encode(commentjson.dumps(credential).encode()).decode() + retrieve_cred_func.return_value = encoded_credential config_file = ConfigFile("User Config", "zowe.config.json", cwd_up_dir_path) - config_file.secure_props = {cwd_up_file_path: credential} + config_file.secure_props = credential config_file.set_secure_props() # Verify the keyring function calls @@ -486,8 +481,9 @@ def test_set_secure_props_exceed_limit(self, delete_pass_func, set_pass_func, ge chunks[-1] += "\0" for index, chunk in enumerate(chunks, start=1): field_name = f"{constants['ZoweAccountName']}-{index}" + service_names = f"{service_name}-{index}" expected_calls.append(mock.call( - service_name, + service_names, field_name, chunk )) From 5046976b00a5c83bca08635106be1ba12077b6b7 Mon Sep 17 00:00:00 2001 From: samadpls Date: Fri, 28 Jul 2023 23:28:20 +0500 Subject: [PATCH 23/45] Updated the encoding to UTF-16 for Windows Signed-off-by: samadpls --- src/core/zowe/core_for_zowe_sdk/config_file.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/core/zowe/core_for_zowe_sdk/config_file.py b/src/core/zowe/core_for_zowe_sdk/config_file.py index b2a04c97..130d00ce 100644 --- a/src/core/zowe/core_for_zowe_sdk/config_file.py +++ b/src/core/zowe/core_for_zowe_sdk/config_file.py @@ -312,7 +312,7 @@ def load_secure_props(self) -> None: if sys.platform == "win32": service_name += "/" + constants["ZoweAccountName"] - secret_value = self._retrieve_password(service_name) + secret_value = self._retrieve_credential(service_name) else: secret_value = keyring.get_password( service_name, constants["ZoweAccountName"] @@ -329,12 +329,12 @@ def load_secure_props(self) -> None: secure_config: str if sys.platform == "win32": - secure_config = secret_value.encode() + secure_config = secret_value.encode("utf-16") else: secure_config = secret_value secure_config_json = commentjson.loads(base64.b64decode(secure_config).decode()) - + # look for credentials stored for currently loaded config try: self.secure_props = secure_config_json.get(self.filepath, {}) From 8de7a6eeb58b58465582584f4145f6ec359aad5d Mon Sep 17 00:00:00 2001 From: samadpls Date: Sun, 30 Jul 2023 22:04:21 +0500 Subject: [PATCH 24/45] Update WIN32_CRED_MAX_STRING_LENGTH value, refactor ConfigFile set_prop and load_prop, and create CredentialManager class Signed-off-by: samadpls --- .../zowe/core_for_zowe_sdk/config_file.py | 126 +++++++---- src/core/zowe/core_for_zowe_sdk/constants.py | 2 +- .../core_for_zowe_sdk/credential_manager.py | 201 ++++++++++++++++++ .../zowe/core_for_zowe_sdk/profile_manager.py | 1 + 4 files changed, 283 insertions(+), 47 deletions(-) create mode 100644 src/core/zowe/core_for_zowe_sdk/credential_manager.py diff --git a/src/core/zowe/core_for_zowe_sdk/config_file.py b/src/core/zowe/core_for_zowe_sdk/config_file.py index 130d00ce..5bdc0996 100644 --- a/src/core/zowe/core_for_zowe_sdk/config_file.py +++ b/src/core/zowe/core_for_zowe_sdk/config_file.py @@ -21,6 +21,7 @@ import commentjson from .constants import constants +from .credential_manager import CredentialManager from .custom_warnings import ( ProfileNotFoundWarning, ProfileParsingWarning, @@ -309,14 +310,12 @@ def load_secure_props(self) -> None: try: service_name = constants["ZoweServiceName"] + is_win32 = sys.platform == "win32" - if sys.platform == "win32": + if is_win32: service_name += "/" + constants["ZoweAccountName"] - secret_value = self._retrieve_credential(service_name) - else: - secret_value = keyring.get_password( - service_name, constants["ZoweAccountName"] - ) + + secret_value = self._retrieve_credential(service_name) # Handle the case when secret_value is None if secret_value is None: @@ -328,13 +327,14 @@ def load_secure_props(self) -> None: ) from exc secure_config: str - if sys.platform == "win32": - secure_config = secret_value.encode("utf-16") - else: - secure_config = secret_value + secure_config = secret_value.encode("utf-16" if is_win32 else "utf-8") + try: + # Decode the credential + secure_config_json = commentjson.loads(base64.b64decode(secure_config).decode("utf-16" if is_win32 else "utf-8")) + except UnicodeDecodeError: + # Fallback to UTF-8 decoding if needed + secure_config_json = commentjson.loads(base64.b64decode(secure_config).decode("utf-8")) - secure_config_json = commentjson.loads(base64.b64decode(secure_config).decode()) - # look for credentials stored for currently loaded config try: self.secure_props = secure_config_json.get(self.filepath, {}) @@ -361,7 +361,7 @@ def _retrieve_credential(self, service_name: str) -> Optional[str]: """ encoded_credential = keyring.get_password(service_name, constants["ZoweAccountName"]) - if encoded_credential is None: + if encoded_credential is None and sys.platform == "win32": # Retrieve the secure value with an index index = 1 temp_value = keyring.get_password(f"{service_name}-{index}", f"{constants['ZoweAccountName']}-{index}") @@ -375,10 +375,39 @@ def _retrieve_credential(self, service_name: str) -> Optional[str]: if encoded_credential is not None and encoded_credential.endswith("\0"): encoded_credential = encoded_credential[:-1] - - + return encoded_credential + def delete_credential(self, service_name: str, account_name: str) -> None: + """ + Delete the credential from the keyring or storage. + If the keyring.delete_password function is not available, iterate through and delete credentials. + Parameters + ---------- + service_name: str + The service name for the credential deletion + account_name: str + The account name for the credential deletion + Returns + ------- + None + """ + + try: + keyring.delete_password(service_name, account_name) + except keyring.errors.PasswordDeleteError: + # keyring.delete_password is not available (e.g., macOS) + if sys.platform == "win32": + # Delete the secure value with an index + index = 1 + while True: + field_name = f"{account_name}-{index}" + try: + keyring.delete_password(f"{service_name}-{index}", field_name) + except keyring.errors.PasswordDeleteError: + break + index += 1 + def set_secure_props(self) -> None: """ Set secure_props for the given config file @@ -388,43 +417,48 @@ def set_secure_props(self) -> None: """ if not HAS_KEYRING: return - + try: + # Filter or suppress specific warning messages + warnings.filterwarnings("ignore", message="^Retrieved an UTF-8 encoded credential") service_name = constants["ZoweServiceName"] - credential = self.secure_props.get(self.filepath, {}) + credential = {self.filepath: self.secure_props} # Check if credential is a non-empty string if credential: + is_win32 = sys.platform == "win32" + if is_win32: + service_name += "/" + constants["ZoweAccountName"] - # Encode the credential - encoded_credential = base64.b64encode(commentjson.dumps(credential).encode()).decode() - if sys.platform == "win32": - service_name += "/" + constants["ZoweAccountName"] - - # Load existing credentials - existing_credential = self._retrieve_credential(service_name) - if existing_credential: - # Decode the existing credential and update secure_props - existing_credential_bytes = base64.b64decode(existing_credential.encode()) - existing_secure_props = commentjson.loads(existing_credential_bytes) - existing_secure_props[self.filepath] = self.secure_props[self.filepath] - encoded_credential = base64.b64encode(commentjson.dumps(existing_secure_props).encode()).decode() - - - if len(encoded_credential) > constants["WIN32_CRED_MAX_STRING_LENGTH"]: - # Split the encoded credential string into chunks of maximum length - keyring.delete_password(service_name, constants["ZoweAccountName"]) - chunk_size = constants["WIN32_CRED_MAX_STRING_LENGTH"] - chunks = [encoded_credential[i : i + chunk_size] for i in range(0, len(encoded_credential), chunk_size)] - # Append NUL byte to the last chunk - chunks[-1] += "\0" - # Set the individual chunks as separate keyring entries - for index, chunk in enumerate(chunks, start=1): - field_name = f"{constants['ZoweAccountName']}-{index}" - keyring.set_password(f"{service_name}-{index}", field_name, chunk) - else: - # Credential length is within the maximum limit, set it as a single keyring entry - keyring.set_password(service_name, constants["ZoweAccountName"],encoded_credential) + + # Load existing credentials, if any + existing_credential = self._retrieve_credential(service_name) + if existing_credential: + + # Decode the existing credential and update secure_props + existing_credential_bytes = base64.b64decode(existing_credential.encode("UTF-16") if is_win32 else existing_credential.encode()) + existing_secure_props = commentjson.loads(existing_credential_bytes) + existing_secure_props[self.filepath].update(credential[self.filepath]) + # Encode the credential + encoded_credential = base64.b64encode(commentjson.dumps(existing_secure_props).encode("UTF-16") if is_win32 else credential.encode()).decode() + # Delete the existing credential + self.delete_credential(service_name , constants["ZoweAccountName"]) + else: + # Encode the credential + encoded_credential = base64.b64encode(commentjson.dumps(credential.encode("UTF-16") if is_win32 else credential.encode())).decode() + # Check if the encoded credential exceeds the maximum length for win32 + if is_win32 and len(encoded_credential) > constants["WIN32_CRED_MAX_STRING_LENGTH"]: + # Split the encoded credential string into chunks of maximum length + chunk_size = constants["WIN32_CRED_MAX_STRING_LENGTH"] + chunks = [encoded_credential[i: i + chunk_size] for i in range(0, len(encoded_credential), chunk_size)] + # Append NUL byte to the last chunk + chunks[-1] += "\0" + # Set the individual chunks as separate keyring entries + for index, chunk in enumerate(chunks, start=1): + field_name = f"{constants['ZoweAccountName']}-{index}" + keyring.set_password(f"{service_name}-{index}", field_name, chunk) + else: + # Credential length is within the maximum limit or not on win32, set it as a single keyring entry keyring.set_password( service_name, constants["ZoweAccountName"], encoded_credential) diff --git a/src/core/zowe/core_for_zowe_sdk/constants.py b/src/core/zowe/core_for_zowe_sdk/constants.py index b56b3da8..8f572f7b 100644 --- a/src/core/zowe/core_for_zowe_sdk/constants.py +++ b/src/core/zowe/core_for_zowe_sdk/constants.py @@ -16,5 +16,5 @@ "ZoweCredentialKey": "Zowe-Plugin", "ZoweServiceName": "Zowe", "ZoweAccountName": "secure_config_props", - "WIN32_CRED_MAX_STRING_LENGTH" : 2560 + "WIN32_CRED_MAX_STRING_LENGTH" : 1280 } diff --git a/src/core/zowe/core_for_zowe_sdk/credential_manager.py b/src/core/zowe/core_for_zowe_sdk/credential_manager.py new file mode 100644 index 00000000..a72d75e4 --- /dev/null +++ b/src/core/zowe/core_for_zowe_sdk/credential_manager.py @@ -0,0 +1,201 @@ +"""Zowe Python Client SDK. + +This program and the accompanying materials are made available under the terms of the +Eclipse Public License v2.0 which accompanies this distribution, and is available at + +https://www.eclipse.org/legal/epl-v20.html + +SPDX-License-Identifier: EPL-2.0 + +Copyright Contributors to the Zowe Project. +""" +import sys, warnings , base64 +from typing import Optional +import commentjson +from zowe.core_for_zowe_sdk import constants +from zowe.core_for_zowe_sdk.exceptions import ( + SecureProfileLoadFailed + ) + +HAS_KEYRING = True +try: + import keyring + +except ImportError: + HAS_KEYRING = False + +class CredentialManager: + _secure_props = {} + + @staticmethod + @property + def secure_props(): + return CredentialManager._secure_props + + @staticmethod + def load_secure_props() -> None: + """ + load secure_props stored for the given config file + Returns + ------- + None + + if keyring is not initialized, set empty value + """ + if not HAS_KEYRING: + CredentialManager._secure_props = {} + return + + try: + service_name = constants["ZoweServiceName"] + is_win32 = sys.platform == "win32" + + if is_win32: + service_name += "/" + constants["ZoweAccountName"] + + secret_value = CredentialManager._retrieve_credential(service_name) + + # Handle the case when secret_value is None + if secret_value is None: + secret_value = "" + + except Exception as exc: + raise SecureProfileLoadFailed( + constants["ZoweServiceName"], error_msg=str(exc) + ) from exc + + secure_config: str + secure_config = secret_value.encode("utf-16" if is_win32 else "utf-8") + try: + # Decode the credential + secure_config_json = commentjson.loads(base64.b64decode(secure_config).decode("utf-16" if is_win32 else "utf-8")) + except UnicodeDecodeError: + # Fallback to UTF-8 decoding if needed + secure_config_json = commentjson.loads(base64.b64decode(secure_config).decode("utf-8")) + + # update the secure props + CredentialManager._secure_props = secure_config_json + + + @staticmethod + def _retrieve_credential(service_name: str) -> Optional[str]: + """ + Retrieve the credential from the keyring or storage. + If the credential exceeds the maximum length, retrieve it in parts. + Parameters + ---------- + service_name: str + The service name for the credential retrieval + Returns + ------- + str + The retrieved encoded credential + """ + encoded_credential = keyring.get_password(service_name, constants["ZoweAccountName"]) + + if encoded_credential is None and sys.platform == "win32": + # Retrieve the secure value with an index + index = 1 + temp_value = keyring.get_password(f"{service_name}-{index}", f"{constants['ZoweAccountName']}-{index}") + while temp_value is not None: + if encoded_credential is None: + encoded_credential = temp_value + else: + encoded_credential += temp_value + index += 1 + temp_value = keyring.get_password(f"{service_name}-{index}", f"{constants['ZoweAccountName']}-{index}") + + if encoded_credential is not None and encoded_credential.endswith("\0"): + encoded_credential = encoded_credential[:-1] + + return encoded_credential + + @staticmethod + def delete_credential(service_name: str, account_name: str) -> None: + """ + Delete the credential from the keyring or storage. + If the keyring.delete_password function is not available, iterate through and delete credentials. + Parameters + ---------- + service_name: str + The service name for the credential deletion + account_name: str + The account name for the credential deletion + Returns + ------- + None + """ + + try: + keyring.delete_password(service_name, account_name) + except keyring.errors.PasswordDeleteError: + # keyring.delete_password is not available (e.g., macOS) + if sys.platform == "win32": + # Delete the secure value with an index + index = 1 + while True: + field_name = f"{account_name}-{index}" + try: + keyring.delete_password(f"{service_name}-{index}", field_name) + except keyring.errors.PasswordDeleteError: + break + index += 1 + + @staticmethod + def save_secure_props()-> None: + """ + Set secure_props for the given config file + Returns + ------- + None + """ + if not HAS_KEYRING: + return + + # Filter or suppress specific warning messages + warnings.filterwarnings("ignore", message="^Retrieved an UTF-8 encoded credential") + service_name = constants["ZoweServiceName"] + credential = CredentialManager._secure_props + # Check if credential is a non-empty string + if credential: + is_win32 = sys.platform == "win32" + if is_win32: + service_name += "/" + constants["ZoweAccountName"] + + + # Load existing credentials, if any + existing_credential = CredentialManager._retrieve_credential(service_name) + if existing_credential: + + # Decode the existing credential and update secure_props + existing_credential_bytes = base64.b64decode(existing_credential.encode("UTF-16") if is_win32 else existing_credential.encode()) + existing_secure_props = commentjson.loads(existing_credential_bytes) + existing_secure_props.update(credential) + # Encode the credential + encoded_credential = base64.b64encode(commentjson.dumps(existing_secure_props).encode("UTF-16") if is_win32 else credential.encode()).decode() + # Delete the existing credential + CredentialManager.delete_credential(service_name , constants["ZoweAccountName"]) + else: + # Encode the credential + encoded_credential = base64.b64encode(commentjson.dumps(credential.encode("UTF-16") if is_win32 else credential.encode())).decode() + # Check if the encoded credential exceeds the maximum length for win32 + if is_win32 and len(encoded_credential) > constants["WIN32_CRED_MAX_STRING_LENGTH"]: + # Split the encoded credential string into chunks of maximum length + chunk_size = constants["WIN32_CRED_MAX_STRING_LENGTH"] + chunks = [encoded_credential[i: i + chunk_size] for i in range(0, len(encoded_credential), chunk_size)] + # Append NUL byte to the last chunk + chunks[-1] += "\0" + # Set the individual chunks as separate keyring entries + for index, chunk in enumerate(chunks, start=1): + field_name = f"{constants['ZoweAccountName']}-{index}" + keyring.set_password(f"{service_name}-{index}", field_name, chunk) + + else: + # Credential length is within the maximum limit or not on win32, set it as a single keyring entry + keyring.set_password( + service_name, constants["ZoweAccountName"], + encoded_credential) + + + + \ No newline at end of file diff --git a/src/core/zowe/core_for_zowe_sdk/profile_manager.py b/src/core/zowe/core_for_zowe_sdk/profile_manager.py index 1e6d49ca..baa429a5 100644 --- a/src/core/zowe/core_for_zowe_sdk/profile_manager.py +++ b/src/core/zowe/core_for_zowe_sdk/profile_manager.py @@ -15,6 +15,7 @@ from typing import Optional from .config_file import ConfigFile, Profile +from .credential_manager import CredentialManager from .custom_warnings import ( ConfigNotFoundWarning, ProfileNotFoundWarning, From 2b0a708af736a360da0294a3d1f2cc54b5f2f4f3 Mon Sep 17 00:00:00 2001 From: samadpls Date: Mon, 31 Jul 2023 00:33:40 +0500 Subject: [PATCH 25/45] updated credential manager Signed-off-by: samadpls --- .../zowe/core_for_zowe_sdk/credential_manager.py | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/core/zowe/core_for_zowe_sdk/credential_manager.py b/src/core/zowe/core_for_zowe_sdk/credential_manager.py index a72d75e4..d2e35922 100644 --- a/src/core/zowe/core_for_zowe_sdk/credential_manager.py +++ b/src/core/zowe/core_for_zowe_sdk/credential_manager.py @@ -57,7 +57,7 @@ def load_secure_props() -> None: # Handle the case when secret_value is None if secret_value is None: - secret_value = "" + return except Exception as exc: raise SecureProfileLoadFailed( @@ -71,7 +71,7 @@ def load_secure_props() -> None: secure_config_json = commentjson.loads(base64.b64decode(secure_config).decode("utf-16" if is_win32 else "utf-8")) except UnicodeDecodeError: # Fallback to UTF-8 decoding if needed - secure_config_json = commentjson.loads(base64.b64decode(secure_config).decode("utf-8")) + secure_config_json = commentjson.loads(base64.b64decode(secure_config).decode()) # update the secure props CredentialManager._secure_props = secure_config_json @@ -92,7 +92,6 @@ def _retrieve_credential(service_name: str) -> Optional[str]: The retrieved encoded credential """ encoded_credential = keyring.get_password(service_name, constants["ZoweAccountName"]) - if encoded_credential is None and sys.platform == "win32": # Retrieve the secure value with an index index = 1 @@ -176,8 +175,9 @@ def save_secure_props()-> None: # Delete the existing credential CredentialManager.delete_credential(service_name , constants["ZoweAccountName"]) else: + print(credential) # Encode the credential - encoded_credential = base64.b64encode(commentjson.dumps(credential.encode("UTF-16") if is_win32 else credential.encode())).decode() + encoded_credential = base64.b64encode(commentjson.dumps(credential).encode("UTF-16") if is_win32 else credential.encode()).decode() # Check if the encoded credential exceeds the maximum length for win32 if is_win32 and len(encoded_credential) > constants["WIN32_CRED_MAX_STRING_LENGTH"]: # Split the encoded credential string into chunks of maximum length @@ -194,8 +194,4 @@ def save_secure_props()-> None: # Credential length is within the maximum limit or not on win32, set it as a single keyring entry keyring.set_password( service_name, constants["ZoweAccountName"], - encoded_credential) - - - - \ No newline at end of file + encoded_credential) \ No newline at end of file From 2ce0dc401b231348dcc3179a0a5931d99a2af338 Mon Sep 17 00:00:00 2001 From: Abdul Samad Siddiqui Date: Mon, 31 Jul 2023 07:42:57 +0000 Subject: [PATCH 26/45] Update credential_manager.py Signed-off-by: Abdul Samad Siddiqui --- src/core/zowe/core_for_zowe_sdk/credential_manager.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/core/zowe/core_for_zowe_sdk/credential_manager.py b/src/core/zowe/core_for_zowe_sdk/credential_manager.py index d2e35922..1d0e4d06 100644 --- a/src/core/zowe/core_for_zowe_sdk/credential_manager.py +++ b/src/core/zowe/core_for_zowe_sdk/credential_manager.py @@ -171,13 +171,13 @@ def save_secure_props()-> None: existing_secure_props = commentjson.loads(existing_credential_bytes) existing_secure_props.update(credential) # Encode the credential - encoded_credential = base64.b64encode(commentjson.dumps(existing_secure_props).encode("UTF-16") if is_win32 else credential.encode()).decode() + encoded_credential = base64.b64encode(commentjson.dumps(existing_secure_props).encode("utf-16" if is_win32 else "utf-8")).decode() # Delete the existing credential CredentialManager.delete_credential(service_name , constants["ZoweAccountName"]) else: print(credential) # Encode the credential - encoded_credential = base64.b64encode(commentjson.dumps(credential).encode("UTF-16") if is_win32 else credential.encode()).decode() + encoded_credential = base64.b64encode(commentjson.dumps(credential).encode("utf-16" if is_win32 else "utf-8")).decode() # Check if the encoded credential exceeds the maximum length for win32 if is_win32 and len(encoded_credential) > constants["WIN32_CRED_MAX_STRING_LENGTH"]: # Split the encoded credential string into chunks of maximum length @@ -194,4 +194,4 @@ def save_secure_props()-> None: # Credential length is within the maximum limit or not on win32, set it as a single keyring entry keyring.set_password( service_name, constants["ZoweAccountName"], - encoded_credential) \ No newline at end of file + encoded_credential) From 1d937827ddf6e9554d7750c30bc170806e229105 Mon Sep 17 00:00:00 2001 From: Abdul Samad Siddiqui Date: Mon, 31 Jul 2023 07:46:11 +0000 Subject: [PATCH 27/45] Update credential_manager.py Signed-off-by: Abdul Samad Siddiqui --- src/core/zowe/core_for_zowe_sdk/credential_manager.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/core/zowe/core_for_zowe_sdk/credential_manager.py b/src/core/zowe/core_for_zowe_sdk/credential_manager.py index 1d0e4d06..ce211433 100644 --- a/src/core/zowe/core_for_zowe_sdk/credential_manager.py +++ b/src/core/zowe/core_for_zowe_sdk/credential_manager.py @@ -65,10 +65,10 @@ def load_secure_props() -> None: ) from exc secure_config: str - secure_config = secret_value.encode("utf-16" if is_win32 else "utf-8") + secure_config = secret_value.encode("UTF-16" if is_win32 else "UTF-8") try: # Decode the credential - secure_config_json = commentjson.loads(base64.b64decode(secure_config).decode("utf-16" if is_win32 else "utf-8")) + secure_config_json = commentjson.loads(base64.b64decode(secure_config).decode("UTF-16" if is_win32 else "UTF-8")) except UnicodeDecodeError: # Fallback to UTF-8 decoding if needed secure_config_json = commentjson.loads(base64.b64decode(secure_config).decode()) @@ -171,13 +171,13 @@ def save_secure_props()-> None: existing_secure_props = commentjson.loads(existing_credential_bytes) existing_secure_props.update(credential) # Encode the credential - encoded_credential = base64.b64encode(commentjson.dumps(existing_secure_props).encode("utf-16" if is_win32 else "utf-8")).decode() + encoded_credential = base64.b64encode(commentjson.dumps(existing_secure_props).encode("UTF-16" if is_win32 else "UTF-8")).decode() # Delete the existing credential CredentialManager.delete_credential(service_name , constants["ZoweAccountName"]) else: print(credential) # Encode the credential - encoded_credential = base64.b64encode(commentjson.dumps(credential).encode("utf-16" if is_win32 else "utf-8")).decode() + encoded_credential = base64.b64encode(commentjson.dumps(credential).encode("UTF-16" if is_win32 else "UTF-8")).decode() # Check if the encoded credential exceeds the maximum length for win32 if is_win32 and len(encoded_credential) > constants["WIN32_CRED_MAX_STRING_LENGTH"]: # Split the encoded credential string into chunks of maximum length From a50d56c8d64a6a57282ba81bb43a8db0bacefc40 Mon Sep 17 00:00:00 2001 From: samadpls Date: Mon, 31 Jul 2023 21:04:23 +0500 Subject: [PATCH 28/45] refactor: defined charset for file Signed-off-by: samadpls --- .../zowe/core_for_zowe_sdk/credential_manager.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/core/zowe/core_for_zowe_sdk/credential_manager.py b/src/core/zowe/core_for_zowe_sdk/credential_manager.py index ce211433..aa83c515 100644 --- a/src/core/zowe/core_for_zowe_sdk/credential_manager.py +++ b/src/core/zowe/core_for_zowe_sdk/credential_manager.py @@ -48,6 +48,7 @@ def load_secure_props() -> None: try: service_name = constants["ZoweServiceName"] + charset = "UTF-16" if is_win32 else "UTF-8" is_win32 = sys.platform == "win32" if is_win32: @@ -65,10 +66,10 @@ def load_secure_props() -> None: ) from exc secure_config: str - secure_config = secret_value.encode("UTF-16" if is_win32 else "UTF-8") + secure_config = secret_value.encode(charset) try: # Decode the credential - secure_config_json = commentjson.loads(base64.b64decode(secure_config).decode("UTF-16" if is_win32 else "UTF-8")) + secure_config_json = commentjson.loads(base64.b64decode(secure_config).decode(charset)) except UnicodeDecodeError: # Fallback to UTF-8 decoding if needed secure_config_json = commentjson.loads(base64.b64decode(secure_config).decode()) @@ -155,6 +156,7 @@ def save_secure_props()-> None: warnings.filterwarnings("ignore", message="^Retrieved an UTF-8 encoded credential") service_name = constants["ZoweServiceName"] credential = CredentialManager._secure_props + charset = charset # Check if credential is a non-empty string if credential: is_win32 = sys.platform == "win32" @@ -167,17 +169,17 @@ def save_secure_props()-> None: if existing_credential: # Decode the existing credential and update secure_props - existing_credential_bytes = base64.b64decode(existing_credential.encode("UTF-16") if is_win32 else existing_credential.encode()) + existing_credential_bytes = base64.b64decode(existing_credential.encode(charset)) existing_secure_props = commentjson.loads(existing_credential_bytes) existing_secure_props.update(credential) # Encode the credential - encoded_credential = base64.b64encode(commentjson.dumps(existing_secure_props).encode("UTF-16" if is_win32 else "UTF-8")).decode() + encoded_credential = base64.b64encode(commentjson.dumps(existing_secure_props).encode(charset)).decode() # Delete the existing credential CredentialManager.delete_credential(service_name , constants["ZoweAccountName"]) else: print(credential) # Encode the credential - encoded_credential = base64.b64encode(commentjson.dumps(credential).encode("UTF-16" if is_win32 else "UTF-8")).decode() + encoded_credential = base64.b64encode(commentjson.dumps(credential).encode(charset)).decode() # Check if the encoded credential exceeds the maximum length for win32 if is_win32 and len(encoded_credential) > constants["WIN32_CRED_MAX_STRING_LENGTH"]: # Split the encoded credential string into chunks of maximum length From d249d1049ac1519f8d0b21bd38e72f5b0adfd2df Mon Sep 17 00:00:00 2001 From: samadpls Date: Mon, 31 Jul 2023 23:12:17 +0500 Subject: [PATCH 29/45] fixed the typo in the config file Signed-off-by: samadpls --- src/core/zowe/core_for_zowe_sdk/config_file.py | 2 +- .../core_for_zowe_sdk/credential_manager.py | 17 +++++++---------- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/src/core/zowe/core_for_zowe_sdk/config_file.py b/src/core/zowe/core_for_zowe_sdk/config_file.py index 5bdc0996..53977a9b 100644 --- a/src/core/zowe/core_for_zowe_sdk/config_file.py +++ b/src/core/zowe/core_for_zowe_sdk/config_file.py @@ -420,7 +420,7 @@ def set_secure_props(self) -> None: try: # Filter or suppress specific warning messages - warnings.filterwarnings("ignore", message="^Retrieved an UTF-8 encoded credential") + warnings.fildterwarnings("ignore", message="^Retrieved an UTF-8 encoded credential") service_name = constants["ZoweServiceName"] credential = {self.filepath: self.secure_props} # Check if credential is a non-empty string diff --git a/src/core/zowe/core_for_zowe_sdk/credential_manager.py b/src/core/zowe/core_for_zowe_sdk/credential_manager.py index aa83c515..26d1be73 100644 --- a/src/core/zowe/core_for_zowe_sdk/credential_manager.py +++ b/src/core/zowe/core_for_zowe_sdk/credential_manager.py @@ -25,12 +25,9 @@ HAS_KEYRING = False class CredentialManager: - _secure_props = {} + secure_props = {} - @staticmethod - @property - def secure_props(): - return CredentialManager._secure_props + @staticmethod def load_secure_props() -> None: @@ -43,13 +40,13 @@ def load_secure_props() -> None: if keyring is not initialized, set empty value """ if not HAS_KEYRING: - CredentialManager._secure_props = {} + CredentialManager.secure_props = {} return try: service_name = constants["ZoweServiceName"] - charset = "UTF-16" if is_win32 else "UTF-8" is_win32 = sys.platform == "win32" + charset = "UTF-16" if is_win32 else "UTF-8" if is_win32: service_name += "/" + constants["ZoweAccountName"] @@ -75,7 +72,7 @@ def load_secure_props() -> None: secure_config_json = commentjson.loads(base64.b64decode(secure_config).decode()) # update the secure props - CredentialManager._secure_props = secure_config_json + CredentialManager.secure_props = secure_config_json @staticmethod @@ -155,11 +152,11 @@ def save_secure_props()-> None: # Filter or suppress specific warning messages warnings.filterwarnings("ignore", message="^Retrieved an UTF-8 encoded credential") service_name = constants["ZoweServiceName"] - credential = CredentialManager._secure_props - charset = charset + credential = CredentialManager.secure_props # Check if credential is a non-empty string if credential: is_win32 = sys.platform == "win32" + charset = "UTF-16" if is_win32 else "UTF-8" if is_win32: service_name += "/" + constants["ZoweAccountName"] From ae629f93d4300f14e52740e56f5d92da4aea2ab1 Mon Sep 17 00:00:00 2001 From: samadpls Date: Thu, 3 Aug 2023 20:06:05 +0500 Subject: [PATCH 30/45] refactor: refactor credential manager Signed-off-by: samadpls --- .../zowe/core_for_zowe_sdk/config_file.py | 33 ++++++++--------- src/core/zowe/core_for_zowe_sdk/constants.py | 2 +- .../core_for_zowe_sdk/credential_manager.py | 36 ++++++++++--------- 3 files changed, 37 insertions(+), 34 deletions(-) diff --git a/src/core/zowe/core_for_zowe_sdk/config_file.py b/src/core/zowe/core_for_zowe_sdk/config_file.py index 53977a9b..80f3adba 100644 --- a/src/core/zowe/core_for_zowe_sdk/config_file.py +++ b/src/core/zowe/core_for_zowe_sdk/config_file.py @@ -327,14 +327,8 @@ def load_secure_props(self) -> None: ) from exc secure_config: str - secure_config = secret_value.encode("utf-16" if is_win32 else "utf-8") - try: - # Decode the credential - secure_config_json = commentjson.loads(base64.b64decode(secure_config).decode("utf-16" if is_win32 else "utf-8")) - except UnicodeDecodeError: - # Fallback to UTF-8 decoding if needed - secure_config_json = commentjson.loads(base64.b64decode(secure_config).decode("utf-8")) - + secure_config = secret_value.encode() + secure_config_json = commentjson.loads(base64.b64decode(secure_config).decode()) # look for credentials stored for currently loaded config try: self.secure_props = secure_config_json.get(self.filepath, {}) @@ -362,6 +356,8 @@ def _retrieve_credential(self, service_name: str) -> Optional[str]: encoded_credential = keyring.get_password(service_name, constants["ZoweAccountName"]) if encoded_credential is None and sys.platform == "win32": + # Filter or suppress specific warning messages + warnings.filterwarnings("ignore", message="^Retrieved an UTF-8 encoded credential") # Retrieve the secure value with an index index = 1 temp_value = keyring.get_password(f"{service_name}-{index}", f"{constants['ZoweAccountName']}-{index}") @@ -376,7 +372,11 @@ def _retrieve_credential(self, service_name: str) -> Optional[str]: if encoded_credential is not None and encoded_credential.endswith("\0"): encoded_credential = encoded_credential[:-1] - return encoded_credential + try: + return encoded_credential.encode('utf-16le').decode() + except (UnicodeDecodeError, AttributeError): + # The credential is not encoded in UTF-16 + return encoded_credential def delete_credential(self, service_name: str, account_name: str) -> None: """ @@ -433,29 +433,30 @@ def set_secure_props(self) -> None: # Load existing credentials, if any existing_credential = self._retrieve_credential(service_name) if existing_credential: - + # Decode the existing credential and update secure_props - existing_credential_bytes = base64.b64decode(existing_credential.encode("UTF-16") if is_win32 else existing_credential.encode()) + existing_credential_bytes = base64.b64decode(existing_credential).decode() existing_secure_props = commentjson.loads(existing_credential_bytes) existing_secure_props[self.filepath].update(credential[self.filepath]) # Encode the credential - encoded_credential = base64.b64encode(commentjson.dumps(existing_secure_props).encode("UTF-16") if is_win32 else credential.encode()).decode() + encoded_credential = base64.b64encode(commentjson.dumps(existing_secure_props).encode()).decode() # Delete the existing credential self.delete_credential(service_name , constants["ZoweAccountName"]) else: + print("here") # Encode the credential - encoded_credential = base64.b64encode(commentjson.dumps(credential.encode("UTF-16") if is_win32 else credential.encode())).decode() + encoded_credential = base64.b64encode(commentjson.dumps(credential).encode()).decode() + print(encoded_credential) # Check if the encoded credential exceeds the maximum length for win32 if is_win32 and len(encoded_credential) > constants["WIN32_CRED_MAX_STRING_LENGTH"]: # Split the encoded credential string into chunks of maximum length chunk_size = constants["WIN32_CRED_MAX_STRING_LENGTH"] chunks = [encoded_credential[i: i + chunk_size] for i in range(0, len(encoded_credential), chunk_size)] - # Append NUL byte to the last chunk - chunks[-1] += "\0" # Set the individual chunks as separate keyring entries for index, chunk in enumerate(chunks, start=1): + password=(chunk + '\0' *(len(chunk)%2)).encode().decode('utf-16le') field_name = f"{constants['ZoweAccountName']}-{index}" - keyring.set_password(f"{service_name}-{index}", field_name, chunk) + keyring.set_password(f"{service_name}-{index}", field_name, password) else: # Credential length is within the maximum limit or not on win32, set it as a single keyring entry diff --git a/src/core/zowe/core_for_zowe_sdk/constants.py b/src/core/zowe/core_for_zowe_sdk/constants.py index 8f572f7b..b56b3da8 100644 --- a/src/core/zowe/core_for_zowe_sdk/constants.py +++ b/src/core/zowe/core_for_zowe_sdk/constants.py @@ -16,5 +16,5 @@ "ZoweCredentialKey": "Zowe-Plugin", "ZoweServiceName": "Zowe", "ZoweAccountName": "secure_config_props", - "WIN32_CRED_MAX_STRING_LENGTH" : 1280 + "WIN32_CRED_MAX_STRING_LENGTH" : 2560 } diff --git a/src/core/zowe/core_for_zowe_sdk/credential_manager.py b/src/core/zowe/core_for_zowe_sdk/credential_manager.py index 26d1be73..89ec1ff4 100644 --- a/src/core/zowe/core_for_zowe_sdk/credential_manager.py +++ b/src/core/zowe/core_for_zowe_sdk/credential_manager.py @@ -46,7 +46,7 @@ def load_secure_props() -> None: try: service_name = constants["ZoweServiceName"] is_win32 = sys.platform == "win32" - charset = "UTF-16" if is_win32 else "UTF-8" + # = "UTF-16" if is_win32 else "UTF-8" if is_win32: service_name += "/" + constants["ZoweAccountName"] @@ -63,13 +63,8 @@ def load_secure_props() -> None: ) from exc secure_config: str - secure_config = secret_value.encode(charset) - try: - # Decode the credential - secure_config_json = commentjson.loads(base64.b64decode(secure_config).decode(charset)) - except UnicodeDecodeError: - # Fallback to UTF-8 decoding if needed - secure_config_json = commentjson.loads(base64.b64decode(secure_config).decode()) + secure_config = secret_value.encode() + secure_config_json = commentjson.loads(base64.b64decode(secure_config).decode()) # update the secure props CredentialManager.secure_props = secure_config_json @@ -91,6 +86,8 @@ def _retrieve_credential(service_name: str) -> Optional[str]: """ encoded_credential = keyring.get_password(service_name, constants["ZoweAccountName"]) if encoded_credential is None and sys.platform == "win32": + # Filter or suppress specific warning messages + warnings.filterwarnings("ignore", message="^Retrieved an UTF-8 encoded credential") # Retrieve the secure value with an index index = 1 temp_value = keyring.get_password(f"{service_name}-{index}", f"{constants['ZoweAccountName']}-{index}") @@ -105,7 +102,12 @@ def _retrieve_credential(service_name: str) -> Optional[str]: if encoded_credential is not None and encoded_credential.endswith("\0"): encoded_credential = encoded_credential[:-1] - return encoded_credential + try: + return encoded_credential.encode('utf-16le').decode() + except (UnicodeDecodeError, AttributeError): + # The credential is not encoded in UTF-16 + return encoded_credential + @staticmethod def delete_credential(service_name: str, account_name: str) -> None: @@ -156,7 +158,7 @@ def save_secure_props()-> None: # Check if credential is a non-empty string if credential: is_win32 = sys.platform == "win32" - charset = "UTF-16" if is_win32 else "UTF-8" + # = "UTF-16" if is_win32 else "UTF-8" if is_win32: service_name += "/" + constants["ZoweAccountName"] @@ -166,28 +168,28 @@ def save_secure_props()-> None: if existing_credential: # Decode the existing credential and update secure_props - existing_credential_bytes = base64.b64decode(existing_credential.encode(charset)) + existing_credential_bytes = base64.b64decode(existing_credential).decode() existing_secure_props = commentjson.loads(existing_credential_bytes) existing_secure_props.update(credential) # Encode the credential - encoded_credential = base64.b64encode(commentjson.dumps(existing_secure_props).encode(charset)).decode() + encoded_credential = base64.b64encode(commentjson.dumps(existing_secure_props).encode()).decode() # Delete the existing credential CredentialManager.delete_credential(service_name , constants["ZoweAccountName"]) else: - print(credential) + print("here") # Encode the credential - encoded_credential = base64.b64encode(commentjson.dumps(credential).encode(charset)).decode() + encoded_credential = base64.b64encode(commentjson.dumps(credential).encode()).decode() + print(encoded_credential) # Check if the encoded credential exceeds the maximum length for win32 if is_win32 and len(encoded_credential) > constants["WIN32_CRED_MAX_STRING_LENGTH"]: # Split the encoded credential string into chunks of maximum length chunk_size = constants["WIN32_CRED_MAX_STRING_LENGTH"] chunks = [encoded_credential[i: i + chunk_size] for i in range(0, len(encoded_credential), chunk_size)] - # Append NUL byte to the last chunk - chunks[-1] += "\0" # Set the individual chunks as separate keyring entries for index, chunk in enumerate(chunks, start=1): + password=(chunk + '\0' *(len(chunk)%2)).encode().decode('utf-16le') field_name = f"{constants['ZoweAccountName']}-{index}" - keyring.set_password(f"{service_name}-{index}", field_name, chunk) + keyring.set_password(f"{service_name}-{index}", field_name, password) else: # Credential length is within the maximum limit or not on win32, set it as a single keyring entry From 07ac8ce2da0097c33e98c56d28506f944f3def96 Mon Sep 17 00:00:00 2001 From: samadpls Date: Thu, 3 Aug 2023 20:14:44 +0500 Subject: [PATCH 31/45] typo fixed Signed-off-by: samadpls --- src/core/zowe/core_for_zowe_sdk/config_file.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/zowe/core_for_zowe_sdk/config_file.py b/src/core/zowe/core_for_zowe_sdk/config_file.py index 80f3adba..cdd30768 100644 --- a/src/core/zowe/core_for_zowe_sdk/config_file.py +++ b/src/core/zowe/core_for_zowe_sdk/config_file.py @@ -316,7 +316,7 @@ def load_secure_props(self) -> None: service_name += "/" + constants["ZoweAccountName"] secret_value = self._retrieve_credential(service_name) - + # Handle the case when secret_value is None if secret_value is None: secret_value = "" @@ -420,7 +420,7 @@ def set_secure_props(self) -> None: try: # Filter or suppress specific warning messages - warnings.fildterwarnings("ignore", message="^Retrieved an UTF-8 encoded credential") + warnings.filterwarnings("ignore", message="^Retrieved an UTF-8 encoded credential") service_name = constants["ZoweServiceName"] credential = {self.filepath: self.secure_props} # Check if credential is a non-empty string From 9d618f51acaaceb32e2141e323a8fc1529c4409f Mon Sep 17 00:00:00 2001 From: samadpls Date: Wed, 9 Aug 2023 00:00:41 +0500 Subject: [PATCH 32/45] added unit tests for the `CredentialManager` class and Addressed comment Signed-off-by: samadpls --- CHANGELOG.md | 2 +- src/core/zowe/core_for_zowe_sdk/__init__.py | 1 + .../zowe/core_for_zowe_sdk/config_file.py | 184 +----------------- .../core_for_zowe_sdk/credential_manager.py | 23 ++- .../zowe/core_for_zowe_sdk/profile_manager.py | 1 - tests/unit/test_zowe_core.py | 169 ++++++++++------ 6 files changed, 132 insertions(+), 248 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c8def386..c1982a99 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,5 +4,5 @@ All notable changes to the Zowe Client Python SDK will be documented in this fil ## Recent Changes -- BugFix: Added a method to load secure values from multiple credential entries on Windows [#134](https://github.com/zowe/zowe-client-python-sdk/issues/134) +- Feature: Added a CredentialManager class to securely retrieve values from credentials and manage multiple credential entries on Windows [#134](https://github.com/zowe/zowe-client-python-sdk/issues/134) - Feature: Added method to load profile properties from environment variables \ No newline at end of file diff --git a/src/core/zowe/core_for_zowe_sdk/__init__.py b/src/core/zowe/core_for_zowe_sdk/__init__.py index ecfd1d22..fb740b4e 100644 --- a/src/core/zowe/core_for_zowe_sdk/__init__.py +++ b/src/core/zowe/core_for_zowe_sdk/__init__.py @@ -12,3 +12,4 @@ from .session import Session from .zosmf_profile import ZosmfProfile from .config_file import ConfigFile +from .credential_manager import CredentialManager diff --git a/src/core/zowe/core_for_zowe_sdk/config_file.py b/src/core/zowe/core_for_zowe_sdk/config_file.py index 5165b7f0..15cb58e9 100644 --- a/src/core/zowe/core_for_zowe_sdk/config_file.py +++ b/src/core/zowe/core_for_zowe_sdk/config_file.py @@ -310,7 +310,8 @@ def load_profile_properties(self, profile_name: str) -> dict: Load exact profile properties (without prepopulated fields from base profile) from the profile dict and populate fields from the secure credentials storage """ - + if self.profiles is None: + self.init_from_file() props = {} lst = profile_name.split(".") secure_fields: list = [] @@ -331,8 +332,8 @@ def load_profile_properties(self, profile_name: str) -> dict: # load secure props only if there are secure fields if secure_fields: - self.load_secure_props() - + CredentialManager.load_secure_props() + self.secure_props=CredentialManager.secure_props.get(self.filepath, {}) # load properties with key as profile.{profile_name}.properties.{*} for (key, value) in self.secure_props.items(): if re.match( @@ -347,180 +348,3 @@ def load_profile_properties(self, profile_name: str) -> dict: # self._missing_secure_props.extend(secure_fields) return props - - def load_secure_props(self) -> None: - """ - load secure_props stored for the given config file - Returns - ------- - None - - if keyring is not initialized, set empty value - """ - if not HAS_KEYRING: - self.secure_props = {} - return - - try: - service_name = constants["ZoweServiceName"] - is_win32 = sys.platform == "win32" - - if is_win32: - service_name += "/" + constants["ZoweAccountName"] - - secret_value = self._retrieve_credential(service_name) - - # Handle the case when secret_value is None - if secret_value is None: - secret_value = "" - - except Exception as exc: - raise SecureProfileLoadFailed( - constants["ZoweServiceName"], error_msg=str(exc) - ) from exc - - secure_config: str - secure_config = secret_value.encode() - secure_config_json = commentjson.loads(base64.b64decode(secure_config).decode()) - # look for credentials stored for currently loaded config - try: - self.secure_props = secure_config_json.get(self.filepath, {}) - except KeyError as exc: - error_msg = str(exc) - warnings.warn( - f"No credentials found for loaded config file '{self.filepath}'" - f" with error '{error_msg}'", - SecurePropsNotFoundWarning, - ) - - def _retrieve_credential(self, service_name: str) -> Optional[str]: - """ - Retrieve the credential from the keyring or storage. - If the credential exceeds the maximum length, retrieve it in parts. - Parameters - ---------- - service_name: str - The service name for the credential retrieval - Returns - ------- - str - The retrieved encoded credential - """ - encoded_credential = keyring.get_password(service_name, constants["ZoweAccountName"]) - - if encoded_credential is None and sys.platform == "win32": - # Filter or suppress specific warning messages - warnings.filterwarnings("ignore", message="^Retrieved an UTF-8 encoded credential") - # Retrieve the secure value with an index - index = 1 - temp_value = keyring.get_password(f"{service_name}-{index}", f"{constants['ZoweAccountName']}-{index}") - while temp_value is not None: - if encoded_credential is None: - encoded_credential = temp_value - else: - encoded_credential += temp_value - index += 1 - temp_value = keyring.get_password(f"{service_name}-{index}", f"{constants['ZoweAccountName']}-{index}") - - if encoded_credential is not None and encoded_credential.endswith("\0"): - encoded_credential = encoded_credential[:-1] - - try: - return encoded_credential.encode('utf-16le').decode() - except (UnicodeDecodeError, AttributeError): - # The credential is not encoded in UTF-16 - return encoded_credential - - def delete_credential(self, service_name: str, account_name: str) -> None: - """ - Delete the credential from the keyring or storage. - If the keyring.delete_password function is not available, iterate through and delete credentials. - Parameters - ---------- - service_name: str - The service name for the credential deletion - account_name: str - The account name for the credential deletion - Returns - ------- - None - """ - - try: - keyring.delete_password(service_name, account_name) - except keyring.errors.PasswordDeleteError: - # keyring.delete_password is not available (e.g., macOS) - if sys.platform == "win32": - # Delete the secure value with an index - index = 1 - while True: - field_name = f"{account_name}-{index}" - try: - keyring.delete_password(f"{service_name}-{index}", field_name) - except keyring.errors.PasswordDeleteError: - break - index += 1 - - def set_secure_props(self) -> None: - """ - Set secure_props for the given config file - Returns - ------- - None - """ - if not HAS_KEYRING: - return - - try: - # Filter or suppress specific warning messages - warnings.filterwarnings("ignore", message="^Retrieved an UTF-8 encoded credential") - service_name = constants["ZoweServiceName"] - credential = {self.filepath: self.secure_props} - # Check if credential is a non-empty string - if credential: - is_win32 = sys.platform == "win32" - if is_win32: - service_name += "/" + constants["ZoweAccountName"] - - - # Load existing credentials, if any - existing_credential = self._retrieve_credential(service_name) - if existing_credential: - - # Decode the existing credential and update secure_props - existing_credential_bytes = base64.b64decode(existing_credential).decode() - existing_secure_props = commentjson.loads(existing_credential_bytes) - existing_secure_props[self.filepath].update(credential[self.filepath]) - # Encode the credential - encoded_credential = base64.b64encode(commentjson.dumps(existing_secure_props).encode()).decode() - # Delete the existing credential - self.delete_credential(service_name , constants["ZoweAccountName"]) - else: - print("here") - # Encode the credential - encoded_credential = base64.b64encode(commentjson.dumps(credential).encode()).decode() - print(encoded_credential) - # Check if the encoded credential exceeds the maximum length for win32 - if is_win32 and len(encoded_credential) > constants["WIN32_CRED_MAX_STRING_LENGTH"]: - # Split the encoded credential string into chunks of maximum length - chunk_size = constants["WIN32_CRED_MAX_STRING_LENGTH"] - chunks = [encoded_credential[i: i + chunk_size] for i in range(0, len(encoded_credential), chunk_size)] - # Set the individual chunks as separate keyring entries - for index, chunk in enumerate(chunks, start=1): - password=(chunk + '\0' *(len(chunk)%2)).encode().decode('utf-16le') - field_name = f"{constants['ZoweAccountName']}-{index}" - keyring.set_password(f"{service_name}-{index}", field_name, password) - - else: - # Credential length is within the maximum limit or not on win32, set it as a single keyring entry - keyring.set_password( - service_name, constants["ZoweAccountName"], - encoded_credential) - - except KeyError as exc: - error_msg = str(exc) - warnings.warn( - f"No credentials found for loaded config file '{self.filepath}'" - f" with error '{error_msg}'", - SecurePropsNotFoundWarning, - ) diff --git a/src/core/zowe/core_for_zowe_sdk/credential_manager.py b/src/core/zowe/core_for_zowe_sdk/credential_manager.py index 89ec1ff4..ef5f96dd 100644 --- a/src/core/zowe/core_for_zowe_sdk/credential_manager.py +++ b/src/core/zowe/core_for_zowe_sdk/credential_manager.py @@ -45,6 +45,14 @@ def load_secure_props() -> None: try: service_name = constants["ZoweServiceName"] + service_name = constants["ZoweServiceName"] + is_win32 = sys.platform == "win32" + # = "UTF-16" if is_win32 else "UTF-8" + + if is_win32: + service_name += "/" + constants["ZoweAccountName"] + + service_name = constants["ZoweServiceName"] is_win32 = sys.platform == "win32" # = "UTF-16" if is_win32 else "UTF-8" @@ -84,8 +92,11 @@ def _retrieve_credential(service_name: str) -> Optional[str]: str The retrieved encoded credential """ + is_win32 = sys.platform == "win32" + if is_win32: + service_name += "/" + constants["ZoweAccountName"] encoded_credential = keyring.get_password(service_name, constants["ZoweAccountName"]) - if encoded_credential is None and sys.platform == "win32": + if encoded_credential is None and is_win32: # Filter or suppress specific warning messages warnings.filterwarnings("ignore", message="^Retrieved an UTF-8 encoded credential") # Retrieve the secure value with an index @@ -126,9 +137,9 @@ def delete_credential(service_name: str, account_name: str) -> None: """ try: - keyring.delete_password(service_name, account_name) + keyring.delete_password(service_name, account_name) except keyring.errors.PasswordDeleteError: - # keyring.delete_password is not available (e.g., macOS) + # Handling multiple credentials stored when the operating system is Windows if sys.platform == "win32": # Delete the secure value with an index index = 1 @@ -162,7 +173,6 @@ def save_secure_props()-> None: if is_win32: service_name += "/" + constants["ZoweAccountName"] - # Load existing credentials, if any existing_credential = CredentialManager._retrieve_credential(service_name) if existing_credential: @@ -176,15 +186,14 @@ def save_secure_props()-> None: # Delete the existing credential CredentialManager.delete_credential(service_name , constants["ZoweAccountName"]) else: - print("here") # Encode the credential encoded_credential = base64.b64encode(commentjson.dumps(credential).encode()).decode() - print(encoded_credential) # Check if the encoded credential exceeds the maximum length for win32 if is_win32 and len(encoded_credential) > constants["WIN32_CRED_MAX_STRING_LENGTH"]: # Split the encoded credential string into chunks of maximum length chunk_size = constants["WIN32_CRED_MAX_STRING_LENGTH"] chunks = [encoded_credential[i: i + chunk_size] for i in range(0, len(encoded_credential), chunk_size)] + chunks[-1]+= '\0' # Set the individual chunks as separate keyring entries for index, chunk in enumerate(chunks, start=1): password=(chunk + '\0' *(len(chunk)%2)).encode().decode('utf-16le') @@ -195,4 +204,4 @@ def save_secure_props()-> None: # Credential length is within the maximum limit or not on win32, set it as a single keyring entry keyring.set_password( service_name, constants["ZoweAccountName"], - encoded_credential) + encoded_credential) \ No newline at end of file diff --git a/src/core/zowe/core_for_zowe_sdk/profile_manager.py b/src/core/zowe/core_for_zowe_sdk/profile_manager.py index 34594d58..70eda0cf 100644 --- a/src/core/zowe/core_for_zowe_sdk/profile_manager.py +++ b/src/core/zowe/core_for_zowe_sdk/profile_manager.py @@ -16,7 +16,6 @@ from typing import Optional from .config_file import ConfigFile, Profile -from .credential_manager import CredentialManager from .custom_warnings import ( ConfigNotFoundWarning, ProfileNotFoundWarning, diff --git a/tests/unit/test_zowe_core.py b/tests/unit/test_zowe_core.py index 0b6306fb..54111440 100644 --- a/tests/unit/test_zowe_core.py +++ b/tests/unit/test_zowe_core.py @@ -5,6 +5,7 @@ import json import os import shutil +import keyring import sys import unittest from unittest import mock @@ -17,6 +18,7 @@ ApiConnection, ConfigFile, ProfileManager, + CredentialManager, RequestHandler, SdkApi, ZosmfProfile, @@ -365,58 +367,110 @@ def test_secure_props_loading_warning(self, get_pass_func): prof_manager = ProfileManager() prof_manager.config_dir = self.custom_dir props: dict = prof_manager.load("base") - - @patch("keyring.get_password", side_effect=keyring_get_password) - def test_profile_not_found_warning(self, get_pass_func): + + @patch("sys.platform", "win32") + @patch("zowe.core_for_zowe_sdk.CredentialManager._retrieve_credential") + def test_load_secure_props(self, retrieve_cred_func): """ - Test correct warnings are being thrown when profile is not found - in config file. - - Only the config folder will be set + Test loading secure_props from keyring or storage. """ - with self.assertWarns(custom_warnings.ProfileNotFoundWarning): - # Setup - custom_file_path = os.path.join(self.custom_dir, "zowe.config.json") - shutil.copy(self.original_file_path, custom_file_path) - - # Test - prof_manager = ProfileManager() - prof_manager.config_dir = self.custom_dir - props: dict = prof_manager.load("non_existent_profile") - + service_name = constants["ZoweServiceName"] + "/" + constants["ZoweAccountName"] + # Setup - copy profile to fake filesystem created by pyfakefs + cwd_up_dir_path = os.path.dirname(CWD) + cwd_up_file_path = os.path.join(cwd_up_dir_path, "zowe.config.json") + os.chdir(CWD) + shutil.copy(self.original_file_path, cwd_up_file_path) + credential = { + cwd_up_file_path: + { + "profiles.base.properties.user": "user", + "profiles.base.properties.password": "password" + } + } + self.setUpCreds(cwd_up_file_path, credential) + base64_encoded_credential = base64.b64encode(commentjson.dumps(credential).encode()).decode() + encoded_credential = base64_encoded_credential.encode('utf-16le').decode() + retrieve_cred_func.return_value = encoded_credential - @patch("keyring.get_password", side_effect=["password", None, "part1","part2\0", None,None,None]) + # call the load_secure_props method + credential_manager = CredentialManager() + credential_manager.load_secure_props() + retrieve_cred_func.assert_called_once_with(service_name) + # Verify the secure_props + expected_secure_props = credential + self.assertEqual(credential_manager.secure_props, expected_secure_props) + + @patch("sys.platform", "win32") + @patch("keyring.delete_password") + def test_delete_credential(self, delete_pass_func): + """ + Test the delete_credential method for deleting credentials from keyring. + """ + def side_effect(*args, **kwargs): + if side_effect.counter < 2: + side_effect.counter += 1 + raise keyring.errors.PasswordDeleteError + else: + return None + side_effect.counter = 0 + + # custom side effect function for the mock + delete_pass_func.side_effect = side_effect + credential_manager = CredentialManager() + service_name = constants['ZoweServiceName'] + account_name = constants['ZoweAccountName'] + # Delete the credential + credential_manager.delete_credential(service_name, account_name) + expected_calls = [ + mock.call(service_name, account_name), + mock.call(f"{service_name}-1", f"{account_name}-1"), + ] + delete_pass_func.assert_has_calls(expected_calls) + + @patch("keyring.get_password", side_effect=["password", None, "part1", "part2\0", None]) def test_retrieve_credential(self, get_pass_func): - # Set up mock values and expected results - # Create a ConfigFile instance and call the method being tested - service_name = "ZoweServiceName" - config_file = ConfigFile("User Config", "test") - retrieve_credential = config_file._retrieve_credential(service_name) + """ + Test the _retrieve_credential method for retrieving credentials from keyring. + """ + credential_manager = CredentialManager() + is_win32 = sys.platform == "win32" + service_name = f"{constants['ZoweServiceName']}/{constants['ZoweAccountName']}" # Scenario 1: Retrieve password directly - expected_password1 = "password" - self.assertEqual(retrieve_credential, expected_password1) + expected_password1 = "password".encode('utf-16le' if is_win32 else "utf-8").decode() + retrieve_credential1 = credential_manager._retrieve_credential(constants['ZoweServiceName']) + self.assertEqual(retrieve_credential1, expected_password1) get_pass_func.assert_called_with(service_name, constants["ZoweAccountName"]) # Scenario 2: Retrieve password in parts - retrieve_credential = config_file._retrieve_credential(service_name) - expected_password2 = "part1part2" - self.assertEqual(retrieve_credential, expected_password2) + expected_password2 = "part1part2".encode('utf-16le' if is_win32 else "utf-8").decode() + retrieve_credential2 = credential_manager._retrieve_credential(constants['ZoweServiceName']) + self.assertEqual(retrieve_credential2, expected_password2) get_pass_func.assert_any_call(service_name, constants["ZoweAccountName"]) get_pass_func.assert_any_call(f"{service_name}-1", f"{constants['ZoweAccountName']}-1") get_pass_func.assert_any_call(f"{service_name}-2", f"{constants['ZoweAccountName']}-2") - # Scenario 3: Password not found - retrieve_credential = config_file._retrieve_credential(service_name) - self.assertIsNone(retrieve_credential) + + @patch("keyring.get_password", side_effect=[None,None]) + def test_retrieve_credential_encoding_errors(self, get_pass_func): + """ + Test the _retrieve_credential method for handling encoding errors and None values. + """ + service_name = f"{constants['ZoweServiceName']}/{constants['ZoweAccountName']}" + result=CredentialManager._retrieve_credential(constants['ZoweServiceName']) + self.assertIsNone(result) get_pass_func.assert_called_with(f"{service_name}-1", f"{constants['ZoweAccountName']}-1") - @mock.patch("sys.platform", "win32") - @mock.patch("keyring.set_password") - @mock.patch("zowe.core_for_zowe_sdk.ConfigFile._retrieve_credential") - def test_set_secure_props_normal_credential(self, retrieve_cred_func, set_pass_func): + + @patch("sys.platform", "win32") + @patch("keyring.set_password") + @patch("zowe.core_for_zowe_sdk.CredentialManager._retrieve_credential") + def test_save_secure_props_normal_credential(self, retrieve_cred_func, set_pass_func): + """ + Test the save_secure_props method for saving credentials to keyring. + """ + # Set up mock values and expected results - service_name =constants["ZoweServiceName"] + "/" + constants["ZoweAccountName"] # Setup - copy profile to fake filesystem created by pyfakefs cwd_up_dir_path = os.path.dirname(CWD) @@ -431,15 +485,11 @@ def test_set_secure_props_normal_credential(self, retrieve_cred_func, set_pass_f } } self.setUpCreds(cwd_up_file_path,credential) - encoded_credential = base64.b64encode(commentjson.dumps(credential).encode()).decode() - - retrieve_cred_func.return_value = encoded_credential + retrieve_cred_func.return_value = None - - config_file = ConfigFile("User Config", "zowe.config.json", cwd_up_dir_path) - config_file.secure_props = credential - config_file.set_secure_props() + CredentialManager.secure_props = credential + CredentialManager.save_secure_props() # Verify the keyring function call set_pass_func.assert_called_once_with( service_name, @@ -447,14 +497,14 @@ def test_set_secure_props_normal_credential(self, retrieve_cred_func, set_pass_f encoded_credential ) - @mock.patch("sys.platform", "win32") - @mock.patch("zowe.core_for_zowe_sdk.ConfigFile._retrieve_credential") - @mock.patch("keyring.set_password") - @mock.patch("keyring.delete_password") - def test_set_secure_props_exceed_limit(self, delete_pass_func, set_pass_func, retrieve_cred_func): + @patch("sys.platform", "win32") + @patch("zowe.core_for_zowe_sdk.CredentialManager._retrieve_credential") + @patch("keyring.set_password") + @patch("zowe.core_for_zowe_sdk.CredentialManager.delete_credential") + def test_save_secure_props_exceed_limit(self, delete_pass_func, set_pass_func, retrieve_cred_func): # Set up mock values and expected results - service_name =constants["ZoweServiceName"] + "/" + constants["ZoweAccountName"] + service_name = constants["ZoweServiceName"] + "/" + constants["ZoweAccountName"] # Setup - copy profile to fake filesystem created by pyfakefs cwd_up_dir_path = os.path.dirname(CWD) cwd_up_file_path = os.path.join(cwd_up_dir_path, "zowe.config.json") @@ -468,32 +518,33 @@ def test_set_secure_props_exceed_limit(self, delete_pass_func, set_pass_func, re } } self.setUpCreds(cwd_up_file_path, credential) - encoded_credential = base64.b64encode(commentjson.dumps(credential).encode()).decode() + base64_encoded_credential = base64.b64encode(commentjson.dumps(credential).encode()).decode() + encoded_credential = base64_encoded_credential.encode('utf-16le').decode() retrieve_cred_func.return_value = encoded_credential - config_file = ConfigFile("User Config", "zowe.config.json", cwd_up_dir_path) - config_file.secure_props = credential - config_file.set_secure_props() + CredentialManager.secure_props = credential + CredentialManager.save_secure_props() - # Verify the keyring function calls - delete_pass_func.assert_called_once_with( - service_name, constants["ZoweAccountName"] - ) + # delete the existing credential + delete_pass_func.return_value = None expected_calls = [] chunk_size = constants["WIN32_CRED_MAX_STRING_LENGTH"] - chunks = [encoded_credential[i: i + chunk_size] for i in range(0, len(encoded_credential), chunk_size)] + chunks = [base64_encoded_credential[i: i + chunk_size] for i in range(0, len(base64_encoded_credential), chunk_size)] chunks[-1] += "\0" for index, chunk in enumerate(chunks, start=1): field_name = f"{constants['ZoweAccountName']}-{index}" service_names = f"{service_name}-{index}" + password=(chunk + '\0' *(len(chunk)%2)).encode().decode('utf-16le') expected_calls.append(mock.call( service_names, field_name, - chunk + password )) set_pass_func.assert_has_calls(expected_calls) + + @patch("keyring.get_password", side_effect=keyring_get_password) def test_profile_loading_with_env_variables(self, get_pass_func): """ From 792ded373ff979c305648c7a5ab182a4a98e5b38 Mon Sep 17 00:00:00 2001 From: samadpls Date: Wed, 9 Aug 2023 00:20:41 +0500 Subject: [PATCH 33/45] added Signed-off-by: samadpls --- src/core/zowe/core_for_zowe_sdk/config_file.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/zowe/core_for_zowe_sdk/config_file.py b/src/core/zowe/core_for_zowe_sdk/config_file.py index 15cb58e9..309d562c 100644 --- a/src/core/zowe/core_for_zowe_sdk/config_file.py +++ b/src/core/zowe/core_for_zowe_sdk/config_file.py @@ -310,8 +310,8 @@ def load_profile_properties(self, profile_name: str) -> dict: Load exact profile properties (without prepopulated fields from base profile) from the profile dict and populate fields from the secure credentials storage """ - if self.profiles is None: - self.init_from_file() + # if self.profiles is None: + # self.init_from_file() props = {} lst = profile_name.split(".") secure_fields: list = [] From a1b9c668e912b3c371970a9911833aa0420f9a85 Mon Sep 17 00:00:00 2001 From: samadpls Date: Wed, 9 Aug 2023 00:27:20 +0500 Subject: [PATCH 34/45] typo fixed Signed-off-by: samadpls --- tests/unit/test_zowe_core.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_zowe_core.py b/tests/unit/test_zowe_core.py index 54111440..c62be481 100644 --- a/tests/unit/test_zowe_core.py +++ b/tests/unit/test_zowe_core.py @@ -427,6 +427,7 @@ def side_effect(*args, **kwargs): ] delete_pass_func.assert_has_calls(expected_calls) + @patch("sys.platform", "win32") @patch("keyring.get_password", side_effect=["password", None, "part1", "part2\0", None]) def test_retrieve_credential(self, get_pass_func): """ @@ -450,7 +451,7 @@ def test_retrieve_credential(self, get_pass_func): get_pass_func.assert_any_call(f"{service_name}-1", f"{constants['ZoweAccountName']}-1") get_pass_func.assert_any_call(f"{service_name}-2", f"{constants['ZoweAccountName']}-2") - + @patch("sys.platform", "win32") @patch("keyring.get_password", side_effect=[None,None]) def test_retrieve_credential_encoding_errors(self, get_pass_func): """ From 6c71b946a1dc24100fd9640a89b864995d88b062 Mon Sep 17 00:00:00 2001 From: samadpls Date: Wed, 9 Aug 2023 18:42:11 +0500 Subject: [PATCH 35/45] typo fixed in load_secure_props() Signed-off-by: samadpls --- .../zowe/core_for_zowe_sdk/credential_manager.py | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/src/core/zowe/core_for_zowe_sdk/credential_manager.py b/src/core/zowe/core_for_zowe_sdk/credential_manager.py index ef5f96dd..af2c5851 100644 --- a/src/core/zowe/core_for_zowe_sdk/credential_manager.py +++ b/src/core/zowe/core_for_zowe_sdk/credential_manager.py @@ -44,23 +44,12 @@ def load_secure_props() -> None: return try: - service_name = constants["ZoweServiceName"] service_name = constants["ZoweServiceName"] is_win32 = sys.platform == "win32" - # = "UTF-16" if is_win32 else "UTF-8" - if is_win32: service_name += "/" + constants["ZoweAccountName"] - - service_name = constants["ZoweServiceName"] - is_win32 = sys.platform == "win32" - # = "UTF-16" if is_win32 else "UTF-8" - - if is_win32: - service_name += "/" + constants["ZoweAccountName"] - + secret_value = CredentialManager._retrieve_credential(service_name) - # Handle the case when secret_value is None if secret_value is None: return @@ -73,7 +62,6 @@ def load_secure_props() -> None: secure_config: str secure_config = secret_value.encode() secure_config_json = commentjson.loads(base64.b64decode(secure_config).decode()) - # update the secure props CredentialManager.secure_props = secure_config_json From 85ea9e07bcc5b2db52cc2ef3fb05fc6796e4e582 Mon Sep 17 00:00:00 2001 From: samadpls Date: Thu, 10 Aug 2023 16:28:47 +0500 Subject: [PATCH 36/45] fixed the typo mistake in `load_secure_prop` Signed-off-by: samadpls --- src/core/zowe/core_for_zowe_sdk/credential_manager.py | 4 ---- tests/unit/test_zowe_core.py | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/src/core/zowe/core_for_zowe_sdk/credential_manager.py b/src/core/zowe/core_for_zowe_sdk/credential_manager.py index af2c5851..0f55e3eb 100644 --- a/src/core/zowe/core_for_zowe_sdk/credential_manager.py +++ b/src/core/zowe/core_for_zowe_sdk/credential_manager.py @@ -45,10 +45,6 @@ def load_secure_props() -> None: try: service_name = constants["ZoweServiceName"] - is_win32 = sys.platform == "win32" - if is_win32: - service_name += "/" + constants["ZoweAccountName"] - secret_value = CredentialManager._retrieve_credential(service_name) # Handle the case when secret_value is None if secret_value is None: diff --git a/tests/unit/test_zowe_core.py b/tests/unit/test_zowe_core.py index c62be481..33c81cdf 100644 --- a/tests/unit/test_zowe_core.py +++ b/tests/unit/test_zowe_core.py @@ -374,7 +374,7 @@ def test_load_secure_props(self, retrieve_cred_func): """ Test loading secure_props from keyring or storage. """ - service_name = constants["ZoweServiceName"] + "/" + constants["ZoweAccountName"] + service_name = constants["ZoweServiceName"] # Setup - copy profile to fake filesystem created by pyfakefs cwd_up_dir_path = os.path.dirname(CWD) cwd_up_file_path = os.path.join(cwd_up_dir_path, "zowe.config.json") From bf4829ea86242bd028e3f0901a67d778594e64ec Mon Sep 17 00:00:00 2001 From: samadpls Date: Fri, 11 Aug 2023 22:08:13 +0500 Subject: [PATCH 37/45] Added condition for non-Windows system Signed-off-by: samadpls --- src/core/zowe/core_for_zowe_sdk/credential_manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/zowe/core_for_zowe_sdk/credential_manager.py b/src/core/zowe/core_for_zowe_sdk/credential_manager.py index 0f55e3eb..806beb8d 100644 --- a/src/core/zowe/core_for_zowe_sdk/credential_manager.py +++ b/src/core/zowe/core_for_zowe_sdk/credential_manager.py @@ -98,7 +98,7 @@ def _retrieve_credential(service_name: str) -> Optional[str]: encoded_credential = encoded_credential[:-1] try: - return encoded_credential.encode('utf-16le').decode() + return encoded_credential.encode('utf-16le' if is_win32 else "utf-8").decode() except (UnicodeDecodeError, AttributeError): # The credential is not encoded in UTF-16 return encoded_credential From 99d3fb1532fc5adf1bbfef1480131da0a18c6216 Mon Sep 17 00:00:00 2001 From: samadpls Date: Sat, 12 Aug 2023 03:09:47 +0500 Subject: [PATCH 38/45] updat the test cases Signed-off-by: samadpls --- .../core_for_zowe_sdk/credential_manager.py | 20 +++++++++++-------- tests/unit/test_zowe_core.py | 2 ++ 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/core/zowe/core_for_zowe_sdk/credential_manager.py b/src/core/zowe/core_for_zowe_sdk/credential_manager.py index 806beb8d..6bcd4dca 100644 --- a/src/core/zowe/core_for_zowe_sdk/credential_manager.py +++ b/src/core/zowe/core_for_zowe_sdk/credential_manager.py @@ -94,14 +94,18 @@ def _retrieve_credential(service_name: str) -> Optional[str]: index += 1 temp_value = keyring.get_password(f"{service_name}-{index}", f"{constants['ZoweAccountName']}-{index}") - if encoded_credential is not None and encoded_credential.endswith("\0"): - encoded_credential = encoded_credential[:-1] + if is_win32: + try: + encoded_credential = encoded_credential.encode('utf-16le').decode() + except (UnicodeDecodeError, AttributeError): + # The credential is not encoded in UTF-16 + pass - try: - return encoded_credential.encode('utf-16le' if is_win32 else "utf-8").decode() - except (UnicodeDecodeError, AttributeError): - # The credential is not encoded in UTF-16 - return encoded_credential + if encoded_credential is not None and encoded_credential.endswith("\0"): + encoded_credential = encoded_credential[:-1] + else: + encoded_credential = encoded_credential.decode() + return encoded_credential @staticmethod @@ -158,7 +162,7 @@ def save_secure_props()-> None: service_name += "/" + constants["ZoweAccountName"] # Load existing credentials, if any - existing_credential = CredentialManager._retrieve_credential(service_name) + existing_credential = CredentialManager._retrieve_credential(constants["ZoweServiceName"]) if existing_credential: # Decode the existing credential and update secure_props diff --git a/tests/unit/test_zowe_core.py b/tests/unit/test_zowe_core.py index 33c81cdf..43a0ac7f 100644 --- a/tests/unit/test_zowe_core.py +++ b/tests/unit/test_zowe_core.py @@ -439,6 +439,7 @@ def test_retrieve_credential(self, get_pass_func): # Scenario 1: Retrieve password directly expected_password1 = "password".encode('utf-16le' if is_win32 else "utf-8").decode() + expected_password1 = expected_password1[:-1] retrieve_credential1 = credential_manager._retrieve_credential(constants['ZoweServiceName']) self.assertEqual(retrieve_credential1, expected_password1) get_pass_func.assert_called_with(service_name, constants["ZoweAccountName"]) @@ -446,6 +447,7 @@ def test_retrieve_credential(self, get_pass_func): # Scenario 2: Retrieve password in parts expected_password2 = "part1part2".encode('utf-16le' if is_win32 else "utf-8").decode() retrieve_credential2 = credential_manager._retrieve_credential(constants['ZoweServiceName']) + retrieve_credential2 = retrieve_credential2[:-1] self.assertEqual(retrieve_credential2, expected_password2) get_pass_func.assert_any_call(service_name, constants["ZoweAccountName"]) get_pass_func.assert_any_call(f"{service_name}-1", f"{constants['ZoweAccountName']}-1") From 6c16930eca27cf978dcc1d765809c541f65bd931 Mon Sep 17 00:00:00 2001 From: samadpls Date: Thu, 17 Aug 2023 02:02:49 +0500 Subject: [PATCH 39/45] Refactored the `save_secure_props` function. Signed-off-by: samadpls --- .../core_for_zowe_sdk/credential_manager.py | 32 ++++++------------- tests/unit/test_zowe_core.py | 14 ++++---- 2 files changed, 18 insertions(+), 28 deletions(-) diff --git a/src/core/zowe/core_for_zowe_sdk/credential_manager.py b/src/core/zowe/core_for_zowe_sdk/credential_manager.py index 6bcd4dca..396a940a 100644 --- a/src/core/zowe/core_for_zowe_sdk/credential_manager.py +++ b/src/core/zowe/core_for_zowe_sdk/credential_manager.py @@ -9,7 +9,10 @@ Copyright Contributors to the Zowe Project. """ -import sys, warnings , base64 +import sys +import warnings +import base64 +import logging from typing import Optional import commentjson from zowe.core_for_zowe_sdk import constants @@ -76,13 +79,13 @@ def _retrieve_credential(service_name: str) -> Optional[str]: str The retrieved encoded credential """ + # Configure the logger to ignore warning messages + logging.getLogger().setLevel(logging.ERROR) is_win32 = sys.platform == "win32" if is_win32: service_name += "/" + constants["ZoweAccountName"] encoded_credential = keyring.get_password(service_name, constants["ZoweAccountName"]) if encoded_credential is None and is_win32: - # Filter or suppress specific warning messages - warnings.filterwarnings("ignore", message="^Retrieved an UTF-8 encoded credential") # Retrieve the secure value with an index index = 1 temp_value = keyring.get_password(f"{service_name}-{index}", f"{constants['ZoweAccountName']}-{index}") @@ -149,39 +152,24 @@ def save_secure_props()-> None: """ if not HAS_KEYRING: return - - # Filter or suppress specific warning messages - warnings.filterwarnings("ignore", message="^Retrieved an UTF-8 encoded credential") + service_name = constants["ZoweServiceName"] credential = CredentialManager.secure_props # Check if credential is a non-empty string if credential: is_win32 = sys.platform == "win32" - # = "UTF-16" if is_win32 else "UTF-8" + + encoded_credential = base64.b64encode(commentjson.dumps(credential).encode()).decode() if is_win32: service_name += "/" + constants["ZoweAccountName"] - - # Load existing credentials, if any - existing_credential = CredentialManager._retrieve_credential(constants["ZoweServiceName"]) - if existing_credential: - - # Decode the existing credential and update secure_props - existing_credential_bytes = base64.b64decode(existing_credential).decode() - existing_secure_props = commentjson.loads(existing_credential_bytes) - existing_secure_props.update(credential) - # Encode the credential - encoded_credential = base64.b64encode(commentjson.dumps(existing_secure_props).encode()).decode() # Delete the existing credential CredentialManager.delete_credential(service_name , constants["ZoweAccountName"]) - else: - # Encode the credential - encoded_credential = base64.b64encode(commentjson.dumps(credential).encode()).decode() # Check if the encoded credential exceeds the maximum length for win32 if is_win32 and len(encoded_credential) > constants["WIN32_CRED_MAX_STRING_LENGTH"]: # Split the encoded credential string into chunks of maximum length chunk_size = constants["WIN32_CRED_MAX_STRING_LENGTH"] + encoded_credential+='\0' chunks = [encoded_credential[i: i + chunk_size] for i in range(0, len(encoded_credential), chunk_size)] - chunks[-1]+= '\0' # Set the individual chunks as separate keyring entries for index, chunk in enumerate(chunks, start=1): password=(chunk + '\0' *(len(chunk)%2)).encode().decode('utf-16le') diff --git a/tests/unit/test_zowe_core.py b/tests/unit/test_zowe_core.py index 43a0ac7f..5cacecfb 100644 --- a/tests/unit/test_zowe_core.py +++ b/tests/unit/test_zowe_core.py @@ -434,18 +434,17 @@ def test_retrieve_credential(self, get_pass_func): Test the _retrieve_credential method for retrieving credentials from keyring. """ credential_manager = CredentialManager() - is_win32 = sys.platform == "win32" service_name = f"{constants['ZoweServiceName']}/{constants['ZoweAccountName']}" # Scenario 1: Retrieve password directly - expected_password1 = "password".encode('utf-16le' if is_win32 else "utf-8").decode() + expected_password1 = "password".encode('utf-16le').decode() expected_password1 = expected_password1[:-1] retrieve_credential1 = credential_manager._retrieve_credential(constants['ZoweServiceName']) self.assertEqual(retrieve_credential1, expected_password1) get_pass_func.assert_called_with(service_name, constants["ZoweAccountName"]) # Scenario 2: Retrieve password in parts - expected_password2 = "part1part2".encode('utf-16le' if is_win32 else "utf-8").decode() + expected_password2 = "part1part2".encode('utf-16le').decode() retrieve_credential2 = credential_manager._retrieve_credential(constants['ZoweServiceName']) retrieve_credential2 = retrieve_credential2[:-1] self.assertEqual(retrieve_credential2, expected_password2) @@ -468,13 +467,14 @@ def test_retrieve_credential_encoding_errors(self, get_pass_func): @patch("sys.platform", "win32") @patch("keyring.set_password") @patch("zowe.core_for_zowe_sdk.CredentialManager._retrieve_credential") - def test_save_secure_props_normal_credential(self, retrieve_cred_func, set_pass_func): + @patch("zowe.core_for_zowe_sdk.CredentialManager.delete_credential") + def test_save_secure_props_normal_credential(self, delete_pass_func, retrieve_cred_func, set_pass_func): """ Test the save_secure_props method for saving credentials to keyring. """ # Set up mock values and expected results - service_name =constants["ZoweServiceName"] + "/" + constants["ZoweAccountName"] + service_name = constants["ZoweServiceName"] + "/" + constants["ZoweAccountName"] # Setup - copy profile to fake filesystem created by pyfakefs cwd_up_dir_path = os.path.dirname(CWD) cwd_up_file_path = os.path.join(cwd_up_dir_path, "zowe.config.json") @@ -483,7 +483,7 @@ def test_save_secure_props_normal_credential(self, retrieve_cred_func, set_pass_ credential = { cwd_up_file_path: { - "profiles.base.properties.user": "user", + "profiles.base.properties.user": "samadpls", "profiles.base.properties.password": "password" } } @@ -493,6 +493,8 @@ def test_save_secure_props_normal_credential(self, retrieve_cred_func, set_pass_ CredentialManager.secure_props = credential CredentialManager.save_secure_props() + # delete the existing credential + delete_pass_func.return_value = None # Verify the keyring function call set_pass_func.assert_called_once_with( service_name, From 39eb6dcc3f2e99a3ad039ee88a83f02781b4b1ac Mon Sep 17 00:00:00 2001 From: samadpls Date: Thu, 17 Aug 2023 19:04:52 +0500 Subject: [PATCH 40/45] updated the test case Signed-off-by: samadpls --- tests/unit/test_zowe_core.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_zowe_core.py b/tests/unit/test_zowe_core.py index 5cacecfb..2ede455e 100644 --- a/tests/unit/test_zowe_core.py +++ b/tests/unit/test_zowe_core.py @@ -524,6 +524,7 @@ def test_save_secure_props_exceed_limit(self, delete_pass_func, set_pass_func, r } self.setUpCreds(cwd_up_file_path, credential) base64_encoded_credential = base64.b64encode(commentjson.dumps(credential).encode()).decode() + base64_encoded_credential+='\0' encoded_credential = base64_encoded_credential.encode('utf-16le').decode() retrieve_cred_func.return_value = encoded_credential @@ -536,7 +537,6 @@ def test_save_secure_props_exceed_limit(self, delete_pass_func, set_pass_func, r expected_calls = [] chunk_size = constants["WIN32_CRED_MAX_STRING_LENGTH"] chunks = [base64_encoded_credential[i: i + chunk_size] for i in range(0, len(base64_encoded_credential), chunk_size)] - chunks[-1] += "\0" for index, chunk in enumerate(chunks, start=1): field_name = f"{constants['ZoweAccountName']}-{index}" service_names = f"{service_name}-{index}" From ceb30932e18f673a8fc8ca0ebb78c5eda3513d9e Mon Sep 17 00:00:00 2001 From: samadpls Date: Tue, 22 Aug 2023 19:58:52 +0500 Subject: [PATCH 41/45] removed unused imports Signed-off-by: samadpls --- src/core/zowe/core_for_zowe_sdk/config_file.py | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/core/zowe/core_for_zowe_sdk/config_file.py b/src/core/zowe/core_for_zowe_sdk/config_file.py index 309d562c..18de845f 100644 --- a/src/core/zowe/core_for_zowe_sdk/config_file.py +++ b/src/core/zowe/core_for_zowe_sdk/config_file.py @@ -10,37 +10,28 @@ Copyright Contributors to the Zowe Project. """ -import base64 import os.path import re import json import requests -import sys import warnings from dataclasses import dataclass, field from typing import Optional, NamedTuple import commentjson -from .constants import constants from .credential_manager import CredentialManager from .custom_warnings import ( ProfileNotFoundWarning, ProfileParsingWarning, - SecurePropsNotFoundWarning, ) -from .exceptions import ProfileNotFound, SecureProfileLoadFailed +from .exceptions import ProfileNotFound from .profile_constants import ( GLOBAL_CONFIG_NAME, TEAM_CONFIG, USER_CONFIG, ) -HAS_KEYRING = True -try: - import keyring -except ImportError: - HAS_KEYRING = False HOME = os.path.expanduser("~") GLOBAl_CONFIG_LOCATION = os.path.join(HOME, ".zowe") From fd729bebcdcb93ede61329516c642a96108d7046 Mon Sep 17 00:00:00 2001 From: samadpls Date: Wed, 23 Aug 2023 14:33:02 +0500 Subject: [PATCH 42/45] removed else condition Signed-off-by: samadpls --- src/core/zowe/core_for_zowe_sdk/credential_manager.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/core/zowe/core_for_zowe_sdk/credential_manager.py b/src/core/zowe/core_for_zowe_sdk/credential_manager.py index 396a940a..53e7b5cb 100644 --- a/src/core/zowe/core_for_zowe_sdk/credential_manager.py +++ b/src/core/zowe/core_for_zowe_sdk/credential_manager.py @@ -106,8 +106,7 @@ def _retrieve_credential(service_name: str) -> Optional[str]: if encoded_credential is not None and encoded_credential.endswith("\0"): encoded_credential = encoded_credential[:-1] - else: - encoded_credential = encoded_credential.decode() + return encoded_credential From 2c1ae288728b1e2ff1bc1c3413560d5e4beb4fd8 Mon Sep 17 00:00:00 2001 From: samadpls Date: Wed, 23 Aug 2023 19:52:24 +0500 Subject: [PATCH 43/45] fixed typo mistakes Signed-off-by: samadpls --- .../zowe/core_for_zowe_sdk/config_file.py | 2 +- .../core_for_zowe_sdk/credential_manager.py | 27 ++++++++++--------- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/core/zowe/core_for_zowe_sdk/config_file.py b/src/core/zowe/core_for_zowe_sdk/config_file.py index 18de845f..6aa99d47 100644 --- a/src/core/zowe/core_for_zowe_sdk/config_file.py +++ b/src/core/zowe/core_for_zowe_sdk/config_file.py @@ -324,7 +324,7 @@ def load_profile_properties(self, profile_name: str) -> dict: # load secure props only if there are secure fields if secure_fields: CredentialManager.load_secure_props() - self.secure_props=CredentialManager.secure_props.get(self.filepath, {}) + self.secure_props = CredentialManager.secure_props.get(self.filepath, {}) # load properties with key as profile.{profile_name}.properties.{*} for (key, value) in self.secure_props.items(): if re.match( diff --git a/src/core/zowe/core_for_zowe_sdk/credential_manager.py b/src/core/zowe/core_for_zowe_sdk/credential_manager.py index 53e7b5cb..656878c3 100644 --- a/src/core/zowe/core_for_zowe_sdk/credential_manager.py +++ b/src/core/zowe/core_for_zowe_sdk/credential_manager.py @@ -127,19 +127,22 @@ def delete_credential(service_name: str, account_name: str) -> None: """ try: - keyring.delete_password(service_name, account_name) + keyring.delete_password(service_name, account_name) except keyring.errors.PasswordDeleteError: - # Handling multiple credentials stored when the operating system is Windows - if sys.platform == "win32": - # Delete the secure value with an index - index = 1 - while True: - field_name = f"{account_name}-{index}" - try: - keyring.delete_password(f"{service_name}-{index}", field_name) - except keyring.errors.PasswordDeleteError: - break - index += 1 + pass + + # Handling multiple credentials stored when the operating system is Windows + if sys.platform == "win32": + index = 1 + while True: + field_name = f"{account_name}-{index}" + service_name = f"{service_name}-{index}" + try: + keyring.delete_password(service_name, field_name) + except keyring.errors.PasswordDeleteError: + break + index += 1 + @staticmethod def save_secure_props()-> None: From 716ef7df8c78b16d7bdc752bbde8ce2b62adae08 Mon Sep 17 00:00:00 2001 From: samadpls Date: Thu, 24 Aug 2023 02:07:12 +0500 Subject: [PATCH 44/45] fixed typo Signed-off-by: samadpls --- tests/unit/test_zowe_core.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_zowe_core.py b/tests/unit/test_zowe_core.py index 2ede455e..6708455a 100644 --- a/tests/unit/test_zowe_core.py +++ b/tests/unit/test_zowe_core.py @@ -200,7 +200,7 @@ def setUpCreds(self, file_path, secure_props): } global SECURE_CONFIG_PROPS - SECURE_CONFIG_PROPS = base64.b64encode((json.dumps(CRED_DICT)).encode("utf-8")) + SECURE_CONFIG_PROPS = base64.b64encode((json.dumps(CRED_DICT)).encode()).decode() if sys.platform == "win32": SECURE_CONFIG_PROPS = SECURE_CONFIG_PROPS.decode("utf-16") From 58178c00b4c33362f7a9ad82ff854763b3dc9823 Mon Sep 17 00:00:00 2001 From: samadpls Date: Thu, 24 Aug 2023 02:10:50 +0500 Subject: [PATCH 45/45] updated setUpCred method Signed-off-by: samadpls --- tests/unit/test_zowe_core.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/unit/test_zowe_core.py b/tests/unit/test_zowe_core.py index 6708455a..d63067ae 100644 --- a/tests/unit/test_zowe_core.py +++ b/tests/unit/test_zowe_core.py @@ -201,8 +201,6 @@ def setUpCreds(self, file_path, secure_props): global SECURE_CONFIG_PROPS SECURE_CONFIG_PROPS = base64.b64encode((json.dumps(CRED_DICT)).encode()).decode() - if sys.platform == "win32": - SECURE_CONFIG_PROPS = SECURE_CONFIG_PROPS.decode("utf-16") @patch("keyring.get_password", side_effect=keyring_get_password) def test_autodiscovery_and_base_profile_loading(self, get_pass_func):