Skip to content

Commit

Permalink
Cleanup of Secrets Redaction
Browse files Browse the repository at this point in the history
Re-introduce assertNotIn and assertIn unit testing
Add simplified unit testing for "simple" password redaction
Fix docstrings and minor errors

Signed-off-by: Elijah Swift <elijah.swift@ibm.com>
  • Loading branch information
ElijahSwiftIBM committed Aug 29, 2023
1 parent 04abaa8 commit c433dce
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 9 deletions.
2 changes: 1 addition & 1 deletion pyracf/common/irrsmo00.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ def call_irrsmo00() -> None:


class IRRSMO00:
"""Interface to irrsmo00 callable service through cpyracf Python extension"""
"""Interface to irrsmo00 callable service through cpyracf Python extension."""

def __init__(self) -> None:
# Initialize size of output buffer
Expand Down
12 changes: 6 additions & 6 deletions pyracf/common/security_admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,11 @@ def __add_field_data(self, field_data: dict):
self._valid_segment_traits[segment] = field_data[segment]

def __overwrite_field_data(self, field_data: dict):
"""Overwrite field data in valid segment traits dictionary"""
"""Overwrite field data in valid segment traits dictionary."""
self._valid_segment_traits = field_data

def __add_additional_secret_traits(self, additional_secret_traits: list):
"""Add additional fields to be redacted in logger output"""
"""Add additional fields to be redacted in logger output."""
for secret in additional_secret_traits:
if secret in self.__secret_traits:
continue
Expand Down Expand Up @@ -161,8 +161,8 @@ def _make_request(
)
self.__clear_state(security_request)
if self.__debug:
# No need to redact anything here since the raw result dictionary
# already has secrets redacted when passed to logger
# No need to redact anything here since the raw result XML
# already has secrets redacted when it is built.
self.__logger.log_xml("Result XML", result_xml)
results = SecurityResult(result_xml)
if self.__debug:
Expand Down Expand Up @@ -226,7 +226,7 @@ def __clear_state(self, security_request: SecurityRequest) -> None:
def __validate_and_add_trait(
self, trait: str, segment: str, value: Union[str, dict]
):
"""Validate the specified trait exists in the specified segment"""
"""Validate the specified trait exists in the specified segment."""
if segment not in self._valid_segment_traits:
return False
if trait not in self._valid_segment_traits[segment]:
Expand Down Expand Up @@ -292,7 +292,7 @@ def _add_traits_directly_to_request_xml_with_no_segments(
def _get_profile(
self, result: Union[dict, bytes], index: int = 0
) -> Union[dict, bytes]:
"Extract the profile section from a result dictionary"
"""Extract the profile section from a result dictionary."""
if self.__generate_requests_only:
# Allows this function to work with "self.__generate_requests_only" mode.
return result
Expand Down
2 changes: 1 addition & 1 deletion pyracf/common/security_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def __init__(self) -> None:
def _get_volume_and_generic_security_definition_values(
self, volume: Union[str, None], generic: bool
) -> None:
"""Get volid and generic xml values for security definition"""
"""Get volid and generic xml values for security definition."""
security_definition_volume = ""
security_definition_generic = "no"
if volume:
Expand Down
2 changes: 1 addition & 1 deletion pyracf/user/user_admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ def take_away_operations_authority(self, userid: str) -> Union[dict, bytes]:
# Auditor Authority
# ============================================================================
def has_auditor_authority(self, userid: str) -> Union[bool, bytes]:
"""Check if a user has auditor authority"""
"""Check if a user has auditor authority."""
profile = self.extract(userid, profile_only=True)
return "auditor" in profile["base"]["attributes"]

Expand Down
12 changes: 12 additions & 0 deletions tests/user/test_user_debug_logging.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ def test_add_user_request_debug_log_passwords_get_redacted_on_success(
self.assertEqual(
success_log, TestUserConstants.TEST_ADD_USER_PASSWORD_SUCCESS_LOG
)
self.assertNotIn(self.test_password, success_log)

def test_add_user_request_debug_log_passwords_get_redacted_on_error(
self,
Expand All @@ -99,6 +100,7 @@ def test_add_user_request_debug_log_passwords_get_redacted_on_error(
pass
error_log = self.ansi_escape.sub("", stdout.getvalue())
self.assertEqual(error_log, TestUserConstants.TEST_ADD_USER_PASSWORD_ERROR_LOG)
self.assertNotIn(self.test_password, error_log)

def test_add_user_request_debug_log_passphrases_get_redacted_on_success(
self,
Expand All @@ -117,6 +119,7 @@ def test_add_user_request_debug_log_passphrases_get_redacted_on_success(
self.assertEqual(
success_log, TestUserConstants.TEST_ADD_USER_PASSPHRASE_SUCCESS_LOG
)
self.assertNotIn(self.test_passphrase, success_log)

def test_add_user_request_debug_log_passphrases_get_redacted_on_error(
self,
Expand All @@ -138,6 +141,7 @@ def test_add_user_request_debug_log_passphrases_get_redacted_on_error(
self.assertEqual(
error_log, TestUserConstants.TEST_ADD_USER_PASSPHRASE_ERROR_LOG
)
self.assertNotIn(self.test_passphrase, error_log)

def test_add_user_request_debug_log_passphrases_and_passwords_get_redacted_on_success(
self,
Expand All @@ -157,6 +161,8 @@ def test_add_user_request_debug_log_passphrases_and_passwords_get_redacted_on_su
success_log,
TestUserConstants.TEST_ADD_USER_PASSPHRASE_AND_PASSWORD_SUCCESS_LOG,
)
self.assertNotIn(self.test_passphrase, success_log)
self.assertNotIn(self.test_password, success_log)

def test_add_user_request_debug_log_passphrases_and_passwords_get_redacted_on_error(
self,
Expand All @@ -178,6 +184,8 @@ def test_add_user_request_debug_log_passphrases_and_passwords_get_redacted_on_er
self.assertEqual(
error_log, TestUserConstants.TEST_ADD_USER_PASSPHRASE_AND_PASSWORD_ERROR_LOG
)
self.assertNotIn(self.test_passphrase, error_log)
self.assertNotIn(self.test_password, error_log)

def test_add_user_request_debug_log_password_xml_tags_not_redacted_on_success(
self,
Expand All @@ -196,6 +204,8 @@ def test_add_user_request_debug_log_password_xml_tags_not_redacted_on_success(
self.assertEqual(
success_log, TestUserConstants.TEST_ADD_USER_PASSWORD_SUCCESS_LOG
)
self.assertEqual(success_log.count("********"), 4)
self.assertIn(self.simple_password, success_log)

def test_add_user_request_debug_log_password_xml_tags_not_redacted_on_error(
self,
Expand All @@ -215,6 +225,8 @@ def test_add_user_request_debug_log_password_xml_tags_not_redacted_on_error(
pass
error_log = self.ansi_escape.sub("", stdout.getvalue())
self.assertEqual(error_log, TestUserConstants.TEST_ADD_USER_PASSWORD_ERROR_LOG)
self.assertEqual(error_log.count("********"), 4)
self.assertIn(self.simple_password, error_log)

# ============================================================================
# Add Additional Secrets
Expand Down
30 changes: 30 additions & 0 deletions tests/user/test_user_result_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,9 @@ def test_user_admin_password_redacted_add_user_success_xml(
result,
TestUserConstants.TEST_ADD_USER_PASSWORD_RESULT_SUCCESS_DICTIONARY,
)
result_str = str(result)
self.assertNotIn(self.test_password, result_str)
self.assertNotIn("(" + " " * len(self.test_password) + ")", result_str)

# Error in environment, SQUIDWRD already added/exists
def test_user_admin_password_redacted_add_user_error_xml(
Expand All @@ -176,6 +179,9 @@ def test_user_admin_password_redacted_add_user_error_xml(
exception.exception.result,
TestUserConstants.TEST_ADD_USER_PASSWORD_RESULT_ERROR_DICTIONARY,
)
result_str = str(exception.exception.result)
self.assertNotIn(self.test_password, result_str)
self.assertNotIn("(" + " " * len(self.test_password) + ")", result_str)

def test_user_admin_passphrase_redacted_add_user_success_xml(
self,
Expand All @@ -192,6 +198,9 @@ def test_user_admin_passphrase_redacted_add_user_success_xml(
result,
TestUserConstants.TEST_ADD_USER_PASSPHRASE_RESULT_SUCCESS_DICTIONARY,
)
result_str = str(result)
self.assertNotIn(self.test_passphrase, result_str)
self.assertNotIn("(" + " " * (len(self.test_passphrase) + 2) + ")", result_str)

# Error in environment, SQUIDWRD already added/exists
def test_user_admin_passphrase_redacted_add_user_error_xml(
Expand All @@ -210,6 +219,9 @@ def test_user_admin_passphrase_redacted_add_user_error_xml(
exception.exception.result,
TestUserConstants.TEST_ADD_USER_PASSPHRASE_RESULT_ERROR_DICTIONARY,
)
result_str = str(exception.exception.result)
self.assertNotIn(self.test_passphrase, result_str)
self.assertNotIn("(" + " " * (len(self.test_passphrase) + 2) + ")", result_str)

def test_user_admin_passphrase_and_password_redacted_add_user_success_xml(
self,
Expand All @@ -226,6 +238,11 @@ def test_user_admin_passphrase_and_password_redacted_add_user_success_xml(
result,
TestUserConstants.TEST_ADD_USER_PASSPHRASE_AND_PASSWORD_RESULT_SUCCESS_DICTIONARY,
)
result_str = str(result)
self.assertNotIn(self.test_passphrase, result_str)
self.assertNotIn(self.test_password, result_str)
self.assertNotIn("(" + " " * (len(self.test_passphrase) + 2) + ")", result_str)
self.assertNotIn("(" + " " * len(self.test_password) + ")", result_str)

def test_user_admin_password_message_not_redacted_add_user_success_xml(
self,
Expand All @@ -242,6 +259,10 @@ def test_user_admin_password_message_not_redacted_add_user_success_xml(
result,
TestUserConstants.TEST_ADD_USER_PASSWORD_RESULT_SUCCESS_DICTIONARY,
)
result_str = str(result)
self.assertNotIn("(" + self.simple_password + ")", result_str)
self.assertNotIn("(" + " " * len(self.simple_password) + ")", result_str)
self.assertIn(self.simple_password, result_str)

# Error in environment, SQUIDWRD already added/exists
def test_user_admin_password_message_not_redacted_add_user_error_xml(
Expand All @@ -260,6 +281,10 @@ def test_user_admin_password_message_not_redacted_add_user_error_xml(
exception.exception.result,
TestUserConstants.TEST_ADD_USER_PASSWORD_RESULT_ERROR_DICTIONARY,
)
result_str = str(exception.exception.result)
self.assertNotIn("(" + self.simple_password + ")", result_str)
self.assertNotIn("(" + " " * len(self.simple_password) + ")", result_str)
self.assertIn(self.simple_password, result_str)

# Error in environment, SQUIDWRD already added/exists
def test_user_admin_passphrase_and_password_redacted_add_user_error_xml(
Expand All @@ -278,6 +303,11 @@ def test_user_admin_passphrase_and_password_redacted_add_user_error_xml(
exception.exception.result,
TestUserConstants.TEST_ADD_USER_PASSPHRASE_AND_PASSWORD_RESULT_ERROR_DICTIONARY,
)
result_str = str(exception.exception.result)
self.assertNotIn(self.test_passphrase, result_str)
self.assertNotIn(self.test_password, result_str)
self.assertNotIn("(" + " " * (len(self.test_passphrase) + 2) + ")", result_str)
self.assertNotIn("(" + " " * len(self.test_password) + ")", result_str)

# ============================================================================
# Delete User
Expand Down

0 comments on commit c433dce

Please sign in to comment.