From 1bc75f6338e720fab753b4cd94938ffc29f034c7 Mon Sep 17 00:00:00 2001 From: israelpolishook Date: Mon, 9 Oct 2023 12:48:17 +0300 Subject: [PATCH 1/5] commit --- .../Integrations/CyberArkPAS/CyberArkPAS.py | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/Packs/CyberArkPAS/Integrations/CyberArkPAS/CyberArkPAS.py b/Packs/CyberArkPAS/Integrations/CyberArkPAS/CyberArkPAS.py index cd90310d1c37..5daeda9723f2 100644 --- a/Packs/CyberArkPAS/Integrations/CyberArkPAS/CyberArkPAS.py +++ b/Packs/CyberArkPAS/Integrations/CyberArkPAS/CyberArkPAS.py @@ -69,6 +69,23 @@ def incident_priority_to_dbot_score(score: float) -> int: return 0 +def normalize_properties_to_dict(properties: str | dict) -> dict: + if not properties: + return {} + if isinstance(properties, dict): + demisto.debug('Properties is already a dict') + return properties + elif isinstance(properties, str): + demisto.debug('Properties is a string, parsing to dict') + try: + properties_dict: dict = json.loads(properties.replace("'", '"')) + return properties_dict + except json.decoder.JSONDecodeError: + raise ValueError("Properties are not valid JSON") + else: + raise ValueError("Properties must be a JSON string or dictionary") + + def filter_by_score(events_data: list, score: int) -> list: if score == 0: return events_data @@ -361,7 +378,7 @@ def add_account(self, safe_name: str, password: str, secret_type: str, - properties: str, + properties: dict, automatic_management_enabled: str, manual_management_reason: str, remote_machines: str, @@ -1018,6 +1035,9 @@ def add_account_command( :param access_restricted_to_remote_machines: Whether or not to restrict access only to specified remote machines. :return: CommandResults """ + demisto.debug(f'Creating account {properties}') + properties = normalize_properties_to_dict(properties) + demisto.debug(f"type of properties is {type(properties)}") response = client.add_account(account_name, address, username, platform_id, safe_name, password, secret_type, properties, automatic_management_enabled, manual_management_reason, remote_machines, access_restricted_to_remote_machines) From 5747cd21851f2766b4453ee6ceb0887df42989aa Mon Sep 17 00:00:00 2001 From: israelpolishook Date: Wed, 18 Oct 2023 14:21:04 +0300 Subject: [PATCH 2/5] add UT and RN --- .../Integrations/CyberArkPAS/CyberArkPAS.py | 25 +++++++++++++----- .../CyberArkPAS/CyberArkPAS_test.py | 26 ++++++++++++++++++- Packs/CyberArkPAS/ReleaseNotes/1_0_14.md | 6 +++++ Packs/CyberArkPAS/pack_metadata.json | 2 +- 4 files changed, 50 insertions(+), 9 deletions(-) create mode 100644 Packs/CyberArkPAS/ReleaseNotes/1_0_14.md diff --git a/Packs/CyberArkPAS/Integrations/CyberArkPAS/CyberArkPAS.py b/Packs/CyberArkPAS/Integrations/CyberArkPAS/CyberArkPAS.py index 5daeda9723f2..d4b68e0ba798 100644 --- a/Packs/CyberArkPAS/Integrations/CyberArkPAS/CyberArkPAS.py +++ b/Packs/CyberArkPAS/Integrations/CyberArkPAS/CyberArkPAS.py @@ -69,7 +69,10 @@ def incident_priority_to_dbot_score(score: float) -> int: return 0 -def normalize_properties_to_dict(properties: str | dict) -> dict: +def order_properties_to_dict(properties: str | dict) -> dict: + """ + ordering the properties so that they are valid json for the api + """ if not properties: return {} if isinstance(properties, dict): @@ -1035,12 +1038,20 @@ def add_account_command( :param access_restricted_to_remote_machines: Whether or not to restrict access only to specified remote machines. :return: CommandResults """ - demisto.debug(f'Creating account {properties}') - properties = normalize_properties_to_dict(properties) - demisto.debug(f"type of properties is {type(properties)}") - response = client.add_account(account_name, address, username, platform_id, safe_name, password, secret_type, - properties, automatic_management_enabled, manual_management_reason, remote_machines, - access_restricted_to_remote_machines) + response = client.add_account( + account_name, + address, + username, + platform_id, + safe_name, + password, + secret_type, + order_properties_to_dict(properties), + automatic_management_enabled, + manual_management_reason, + remote_machines, + access_restricted_to_remote_machines, + ) results = CommandResults( raw_response=response, outputs_prefix='CyberArkPAS.Accounts', diff --git a/Packs/CyberArkPAS/Integrations/CyberArkPAS/CyberArkPAS_test.py b/Packs/CyberArkPAS/Integrations/CyberArkPAS/CyberArkPAS_test.py index d9e403f67113..4611d24c1b55 100644 --- a/Packs/CyberArkPAS/Integrations/CyberArkPAS/CyberArkPAS_test.py +++ b/Packs/CyberArkPAS/Integrations/CyberArkPAS/CyberArkPAS_test.py @@ -4,7 +4,7 @@ update_user_command, add_safe_command, update_safe_command, get_list_safes_command, get_safe_by_name_command, \ add_safe_member_command, update_safe_member_command, list_safe_members_command, add_account_command, \ update_account_command, get_list_accounts_command, get_list_account_activity_command, fetch_incidents, \ - get_account_details_command + get_account_details_command, order_properties_to_dict from test_data.context import ADD_USER_CONTEXT, GET_USERS_CONTEXT, \ UPDATE_USER_CONTEXT, UPDATE_SAFE_CONTEXT, GET_LIST_SAFES_CONTEXT, GET_SAFE_BY_NAME_CONTEXT, ADD_SAFE_CONTEXT, \ ADD_SAFE_MEMBER_CONTEXT, UPDATE_SAFE_MEMBER_CONTEXT, LIST_SAFE_MEMBER_CONTEXT, ADD_ACCOUNT_CONTEXT, \ @@ -487,3 +487,27 @@ def test_pas_safe_members_list(mocker, response: dict, count_or_total: str, expe results = list_safe_members_command(client=client, safe_name="Test") assert count_or_total in results.readable_output assert results.outputs == expected_output + + +@pytest.mark.parametrize("properties,expected", [ + ({}, {}), + ({"a": 1}, {"a": 1}), + ({'a': 1}, {"a": 1}), + ("{'a': 1}", {"a": 1}), + ('{"a": 1}', {"a": 1}), + ("not json", ValueError) +]) +def test_order_properties_to_dict(properties, expected): + """ + Given: + - properties as input + When: + - order_properties_to_dict is called on properties + Then: + - it returns the expected output + """ + if not isinstance(expected, dict): + with pytest.raises(expected): + order_properties_to_dict(properties) + else: + assert order_properties_to_dict(properties) == expected \ No newline at end of file diff --git a/Packs/CyberArkPAS/ReleaseNotes/1_0_14.md b/Packs/CyberArkPAS/ReleaseNotes/1_0_14.md new file mode 100644 index 000000000000..d5a371d926a8 --- /dev/null +++ b/Packs/CyberArkPAS/ReleaseNotes/1_0_14.md @@ -0,0 +1,6 @@ + +#### Integrations + +##### CyberArk PAS + +- Fixed an issue where the ***cyberark-pas-account-add*** command failed when run with *properties* argument. diff --git a/Packs/CyberArkPAS/pack_metadata.json b/Packs/CyberArkPAS/pack_metadata.json index 3b39d83602a4..34a64bdc8e34 100644 --- a/Packs/CyberArkPAS/pack_metadata.json +++ b/Packs/CyberArkPAS/pack_metadata.json @@ -2,7 +2,7 @@ "name": "CyberArk", "description": "Provides a Safe Haven, where all your administrative passwords can be securely archived, transferred and shared by authorized users.", "support": "xsoar", - "currentVersion": "1.0.13", + "currentVersion": "1.0.14", "author": "Cortex XSOAR", "url": "https://www.paloaltonetworks.com/cortex", "email": "", From d687e7f8e815dfeb496978cd1180020a6243767a Mon Sep 17 00:00:00 2001 From: israelpolishook Date: Wed, 18 Oct 2023 14:23:38 +0300 Subject: [PATCH 3/5] corrections --- Packs/CyberArkPAS/Integrations/CyberArkPAS/CyberArkPAS.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/Packs/CyberArkPAS/Integrations/CyberArkPAS/CyberArkPAS.py b/Packs/CyberArkPAS/Integrations/CyberArkPAS/CyberArkPAS.py index d4b68e0ba798..210b4e1d83ee 100644 --- a/Packs/CyberArkPAS/Integrations/CyberArkPAS/CyberArkPAS.py +++ b/Packs/CyberArkPAS/Integrations/CyberArkPAS/CyberArkPAS.py @@ -76,10 +76,8 @@ def order_properties_to_dict(properties: str | dict) -> dict: if not properties: return {} if isinstance(properties, dict): - demisto.debug('Properties is already a dict') return properties elif isinstance(properties, str): - demisto.debug('Properties is a string, parsing to dict') try: properties_dict: dict = json.loads(properties.replace("'", '"')) return properties_dict From 4a62e2628dd90957a4dabc5c5cddb2d56afa8ecf Mon Sep 17 00:00:00 2001 From: israelpolishook Date: Wed, 18 Oct 2023 14:38:50 +0300 Subject: [PATCH 4/5] Fix DS108 and update Docker --- .../Integrations/CyberArkPAS/CyberArkPAS.yml | 20 +++++++++---------- Packs/CyberArkPAS/ReleaseNotes/1_0_14.md | 2 +- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/Packs/CyberArkPAS/Integrations/CyberArkPAS/CyberArkPAS.yml b/Packs/CyberArkPAS/Integrations/CyberArkPAS/CyberArkPAS.yml index fa5a5e6e7a6a..09ca5a842319 100644 --- a/Packs/CyberArkPAS/Integrations/CyberArkPAS/CyberArkPAS.yml +++ b/Packs/CyberArkPAS/Integrations/CyberArkPAS/CyberArkPAS.yml @@ -68,7 +68,7 @@ script: required: true - description: The user type according to the license. name: user_type - - description: The CyberArkPAS interfaces that this user is not authorized to use, e.g., - "PSM", "PSMP" + - description: The CyberArkPAS interfaces that this user is not authorized to use, e.g., - "PSM", "PSMP". isArray: true name: non_authorized_interfaces - description: The location in the vault where the user will be created. Must begin with "\\". If just "\\", the vault is in the root. @@ -93,7 +93,7 @@ script: predefined: - 'true' - 'false' - - description: 'A comma-separated list of user permissions. Valid values are: AuditUsers, AddUpdateUsers, ResetUsersPasswords, ActivateUsers, AddNetworkAreas, ManageDirectoryMapping, ManageServerFileCategories, BackupAllSafes, RestoreAllSafes e.g., AddSafes,AuditUsers' + - description: 'A comma-separated list of user permissions. Valid values are: AuditUsers, AddUpdateUsers, ResetUsersPasswords, ActivateUsers, AddNetworkAreas, ManageDirectoryMapping, ManageServerFileCategories, BackupAllSafes, RestoreAllSafes e.g., AddSafes,AuditUsers.' isArray: true name: vault_authorization - description: Notes and comments. @@ -165,7 +165,7 @@ script: name: username - description: User type according to the license. name: user_type - - description: The CyberArkPAS interfaces that this user is not authorized to use, e.g., "PSM", "PSMP" + - description: The CyberArkPAS interfaces that this user is not authorized to use, e.g., "PSM", "PSMP". isArray: true name: non_authorized_interfaces - description: The location in the vault where the user will be created. Must begin with "\\". If just "\\", the vault is in the root. @@ -186,7 +186,7 @@ script: predefined: - 'true' - 'false' - - description: 'A comma-separated list of user permissions. Valid values are: AddSafes, AuditUsers, AddUpdateUsers, ResetUsersPasswords, ActivateUsers, AddNetworkAreas, ManageDirectoryMapping, ManageServerFileCategories, BackupAllSafes, RestoreAllSafes e.g., AddSafes,AuditUsers' + - description: 'A comma-separated list of user permissions. Valid values are: AddSafes, AuditUsers, AddUpdateUsers, ResetUsersPasswords, ActivateUsers, AddNetworkAreas, ManageDirectoryMapping, ManageServerFileCategories, BackupAllSafes, RestoreAllSafes e.g., AddSafes,AuditUsers.' isArray: true name: vault_authorization - description: Notes and comments. @@ -510,7 +510,7 @@ script: - description: |- The user’s permissions in the safe. Valid values: UseAccounts, RetrieveAccounts, ListAccounts, AddAccounts, UpdateAccountContent, UpdateAccountProperties, InitiateCPMAccountManagementOperations, InitiateCPMAccountManagementOperations, SpecifyNextAccountContent, RenameAccounts, DeleteAccounts, UnlockAccounts, ManageSafe, ManageSafeMembers, BackupSafe, ViewAuditLog, ViewAuditLog, ViewSafeMembers, AccessWithoutConfirmation, CreateFolders, DeleteFolders, MoveAccountsAndFolders - e.g., UseAccounts,RetrieveAccounts + e.g., UseAccounts,RetrieveAccounts. isArray: true name: permissions - description: The name of the safe to add a member to. @@ -554,7 +554,7 @@ script: The user’s permissions in the safe. Valid values are: UseAccounts, RetrieveAccounts, ListAccounts, AddAccounts, UpdateAccountContent, UpdateAccountProperties, InitiateCPMAccountManagementOperations, InitiateCPMAccountManagementOperations, SpecifyNextAccountContent, RenameAccounts, DeleteAccounts, UnlockAccounts, ManageSafe, ManageSafeMembers, BackupSafe, ViewAuditLog, ViewAuditLog, ViewSafeMembers, RequestsAuthorizationLevel, AccessWithoutConfirmation, CreateFolders, DeleteFolders, MoveAccountsAndFolders - e.g., UseAccounts,RetrieveAccounts + e.g., UseAccounts,RetrieveAccounts. isArray: true name: permissions - description: The name of the safe to which the safe member belongs. @@ -631,7 +631,7 @@ script: secret: true - description: |- Object containing key-value pairs to associate with the account, as defined by the account platform. - e.g., {"Location": "IT", "OwnerName": "MSSPAdmin"} + e.g., {"Location": "IT", "OwnerName": "MSSPAdmin"}. name: properties - auto: PREDEFINED defaultValue: 'true' @@ -745,12 +745,12 @@ script: - arguments: - description: |- List of keywords to search for in the accounts. - Separated with a space, e.g,. Windows admin + Separated with a space, e.g,. Windows admin. name: search - description: |- Property or properties by which to sort the returned accounts. The properties are followed by a comma and then 'asc' (default) or 'desc' to control the sort direction, - e.g., Windows,asc + e.g., Windows,asc. name: sort - defaultValue: '0' description: The offset of the first account that is returned in the collection of results. Default is '0'. @@ -932,7 +932,7 @@ script: - contextPath: CyberArkPAS.SecurityEvents.type description: The type of the security events. type: String - dockerimage: demisto/python3:3.10.12.63474 + dockerimage: demisto/python3:3.10.13.77674 isfetch: true runonce: false script: '-' diff --git a/Packs/CyberArkPAS/ReleaseNotes/1_0_14.md b/Packs/CyberArkPAS/ReleaseNotes/1_0_14.md index d5a371d926a8..e58dc2ec3860 100644 --- a/Packs/CyberArkPAS/ReleaseNotes/1_0_14.md +++ b/Packs/CyberArkPAS/ReleaseNotes/1_0_14.md @@ -2,5 +2,5 @@ #### Integrations ##### CyberArk PAS - +- Updated the Docker image to: *demisto/python3:3.10.13.77674*. - Fixed an issue where the ***cyberark-pas-account-add*** command failed when run with *properties* argument. From ed17c226db1a8d52bac7a7305ed0917ea414044b Mon Sep 17 00:00:00 2001 From: israelpolishook Date: Wed, 18 Oct 2023 15:18:45 +0300 Subject: [PATCH 5/5] correction comments --- .../Integrations/CyberArkPAS/CyberArkPAS.py | 9 ++++--- .../CyberArkPAS/CyberArkPAS_test.py | 24 ++++++++++++++----- 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/Packs/CyberArkPAS/Integrations/CyberArkPAS/CyberArkPAS.py b/Packs/CyberArkPAS/Integrations/CyberArkPAS/CyberArkPAS.py index 210b4e1d83ee..c27ef125a230 100644 --- a/Packs/CyberArkPAS/Integrations/CyberArkPAS/CyberArkPAS.py +++ b/Packs/CyberArkPAS/Integrations/CyberArkPAS/CyberArkPAS.py @@ -79,12 +79,11 @@ def order_properties_to_dict(properties: str | dict) -> dict: return properties elif isinstance(properties, str): try: - properties_dict: dict = json.loads(properties.replace("'", '"')) - return properties_dict + return json.loads(properties.replace("'", '"')) except json.decoder.JSONDecodeError: - raise ValueError("Properties are not valid JSON") + raise ValueError(f"Properties ({properties}) are not valid JSON") else: - raise ValueError("Properties must be a JSON string or dictionary") + raise ValueError(f"Properties must be a JSON string or dictionary (got {properties})") def filter_by_score(events_data: list, score: int) -> list: @@ -1013,7 +1012,7 @@ def add_account_command( safe_name: str = "", password: str = "", secret_type: str = "password", - properties: str = "", + properties: dict | str = "", automatic_management_enabled: str = "true", manual_management_reason: str = "", remote_machines: str = "", diff --git a/Packs/CyberArkPAS/Integrations/CyberArkPAS/CyberArkPAS_test.py b/Packs/CyberArkPAS/Integrations/CyberArkPAS/CyberArkPAS_test.py index 4611d24c1b55..8d5af44f0d8f 100644 --- a/Packs/CyberArkPAS/Integrations/CyberArkPAS/CyberArkPAS_test.py +++ b/Packs/CyberArkPAS/Integrations/CyberArkPAS/CyberArkPAS_test.py @@ -495,7 +495,7 @@ def test_pas_safe_members_list(mocker, response: dict, count_or_total: str, expe ({'a': 1}, {"a": 1}), ("{'a': 1}", {"a": 1}), ('{"a": 1}', {"a": 1}), - ("not json", ValueError) + ("", {}) ]) def test_order_properties_to_dict(properties, expected): """ @@ -506,8 +506,20 @@ def test_order_properties_to_dict(properties, expected): Then: - it returns the expected output """ - if not isinstance(expected, dict): - with pytest.raises(expected): - order_properties_to_dict(properties) - else: - assert order_properties_to_dict(properties) == expected \ No newline at end of file + assert order_properties_to_dict(properties) == expected + + +@pytest.mark.parametrize("properties,expected", [ + ("not json", ValueError), +]) +def test_order_properties_to_dict_failure(properties, expected): + """ + Given: + - Invalid properties as input + When: + - order_properties_to_dict is called on properties + Then: + - An error is raised + """ + with pytest.raises(expected): + order_properties_to_dict(properties)