Addressed PR Comments

-Moved Debug logging testing of secret traits to common
-Re-architected racf_key_map and racf_message_key_map under 1 dictionary
-changed comments
-established SecretRedactionError

Signed-off-by: Elijah Swift <elijah.swift@ibm.com>

pyracf/common/exceptions/secrets_redaction_error.py

@@ -0,0 +1,35 @@
+"""Exception to use when Additional Secrets could not be Redacted."""
+
+from typing import List
+
+
+class SecretsRedactionError(Exception):
+    """
+    Raised when a specified secret cannot be redacted because it does not map to a segement:trait.
+    """
+
+    def __init__(
+        self, profile_type: str = "", bad_secret_traits: List[str] = []
+    ) -> None:
+        profile_map = {
+            "user": "User",
+            "group": "Group",
+            "dataSet": "Data Set",
+            "resource": "General Resource",
+            "permission": "Access",
+            "groupConnection": "Group Connection",
+            "systemSettings": "Setropts",
+        }
+        self.message = (
+            f"Cannot add specified additional secrets to {profile_map[profile_type]} "
+            + "administration."
+        )
+
+        if bad_secret_traits:
+            for trait in bad_secret_traits:
+                self.message = self.message + (
+                    f"\nCould not map {trait} to a valid segment trait."
+                )
+
+    def __str__(self) -> str:
+        return self.message

pyracf/common/security_admin.py

@@ -7,6 +7,7 @@
 from typing import Any, List, Tuple, Union
 
 from .exceptions.downstream_fatal_error import DownstreamFatalError
+from .exceptions.secrets_redaction_error import SecretsRedactionError
 from .exceptions.security_request_error import SecurityRequestError
 from .exceptions.segment_error import SegmentError
 from .exceptions.segment_trait_error import SegmentTraitError
@@ -54,10 +55,8 @@ class SecurityAdmin:
 
     _valid_segment_traits = {}
     _extracted_key_value_pair_segment_traits_map = {}
-    _racf_key_map = {}
-    _racf_message_key_map = {}
-    _racf_key_map = {}
-    _racf_message_key_map = {}
+    # Use this structure to map traits to their RACF keywords and Message keywords for redaction
+    _racf_trait_and_message_key_map = {"trait_map": {}, "message_map": {}}
     _case_sensitive_extracted_values = []
     __running_userid = None
     _logger = Logger()
@@ -177,8 +176,7 @@ def __raw_dump(self) -> None:
             self._logger.log_hex_dump(
                 raw_result_xml,
                 self.__secret_traits,
-                self._racf_key_map,
-                self._racf_message_key_map,
+                self._racf_trait_and_message_key_map,
             )
 
     # ============================================================================
@@ -188,18 +186,26 @@ def __add_additional_secret_traits(self, additional_secret_traits: list) -> None
         """Add additional fields to be redacted in logger output."""
         unsupported_profile_types = ["permission", "groupConnection", "systemSettings"]
         if self._profile_type in unsupported_profile_types:
-            return
+            raise SecretsRedactionError(profile_type=self._profile_type)
+        bad_secret_traits = []
         for secret in additional_secret_traits:
             if secret in self.__secret_traits:
                 continue
             if ":" not in secret:
+                bad_secret_traits.append(secret)
                 continue
             segment = secret.split(":")[0]
             if segment not in self._valid_segment_traits:
+                bad_secret_traits.append(secret)
                 continue
             if secret not in self._valid_segment_traits[segment]:
+                bad_secret_traits.append(secret)
                 continue
             self.__secret_traits[secret] = self._valid_segment_traits[segment][secret]
+        if bad_secret_traits:
+            raise SecretsRedactionError(
+                profile_type=self._profile_type, bad_secret_traits=bad_secret_traits
+            )
 
     # ============================================================================
     # Request Execution
@@ -254,8 +260,7 @@ def _make_request(
                 self.__running_userid,
             ),
             self.__secret_traits,
-            self._racf_key_map,
-            self._racf_message_key_map,
+            self._racf_trait_and_message_key_map,
         )
         self.__clear_state(security_request)
         if isinstance(raw_result, list):

pyracf/common/utilities/logger.py

@@ -168,10 +168,10 @@ def __redact_string(
         This is designed to match the pattern TRAIT( subtrait1(value) subtrait2(value)) by
         matching the TRAIT name, a variable (potentially zero) amount of spaces, then the
         ( subtrait1(value) subtrait2(value)) portion which must start and end with ( and ),
-        but must also contain a nested set of opened parentheses rather than a direct seqence of
-        them. The pattern looks for these nested open parenthesis as a sequence would have a )
-        character between them. Then the expression allows any non-newline characters and the
-        "end pattern" of ) and ) separated by a variable (potentially zero) whitespace.
+        but must also contain a nested set of open and close parentheses rather than a direct
+        seqence of them. The pattern looks for these nested open parenthesis as a sequence would
+        have a ) character between them. Then the expression allows any non-newline characters and
+        the "end pattern" of ) and ) separated by a variable (potentially zero) whitespace.
 
         If neither of these two patterns is found for a supplied trait_key, then it is assumed
         this trait is set with the default pattern below.
@@ -180,7 +180,7 @@ def __redact_string(
         This is designed to match the pattern TRAIT(value) by matching the TRAIT name, a variable
         (potentially zero) amount of spaces, then the (value) portion which must start and end
         with ( and ), but can conceivably contain any characters, but a negative lookbehind
-        is used to look for any escape \ character that would indicate the matching of the
+        is used to look for any escape character `\` that would indicate the matching of the
         ( and ) is otherwise a coincidence.
 
         In all replacement expressions, the variable amounts of whitespace are captured so that
@@ -189,32 +189,33 @@ def __redact_string(
         """
         asterisks = "********"
         is_bytes = False
+        quoted_regex = rf"{trait_key.upper()}( +){{0,}}\(\'.*?(?<!\')\'\)"
+        nested_regex = rf"{trait_key.upper()}( +){{0,}}\([^)]*\(.*\)( +){{0,}}\)"
+        default_regex = rf"{trait_key.upper()}( +){{0,}}\(.*?(?<!\\)\)"
+        redacted_regex = rf"{trait_key.upper()}\1({asterisks})"
+        redacted_w_quotes_regex = rf"{trait_key.upper()}\1('{asterisks}')"
         if isinstance(input_string, bytes):
             input_string = input_string.decode("cp1047")
             is_bytes = True
-        quoted = re.search(
-            rf"{trait_key.upper()}( +){{0,}}\(\'.*?(?<!\')\'\)", input_string
-        )
-        nested = re.search(
-            rf"{trait_key.upper()}( +){{0,}}\([^)]*\(.*\)( +){{0,}}\)", input_string
-        )
+        quoted = re.search(quoted_regex, input_string)
+        nested = re.search(nested_regex, input_string)
         if quoted is not None:
             input_string = re.sub(
-                rf"{trait_key.upper()}( +){{0,}}\(\'.*?(?<!\')\'\)",
-                rf"{trait_key.upper()}\1('{asterisks}')",
+                quoted_regex,
+                redacted_w_quotes_regex,
                 input_string,
             )
         else:
             if nested is not None:
                 input_string = re.sub(
-                    rf"{trait_key.upper()}( +){{0,}}\([^)]*\(.*\)( +){{0,}}\)",
-                    rf"{trait_key.upper()}\1({asterisks})",
+                    nested_regex,
+                    redacted_regex,
                     input_string,
                 )
             else:
                 input_string = re.sub(
-                    rf"{trait_key.upper()}( +){{0,}}\(.*?(?<!\\)\)",
-                    rf"{trait_key.upper()}\1({asterisks})",
+                    default_regex,
+                    redacted_regex,
                     input_string,
                 )
         if is_bytes:
@@ -234,10 +235,10 @@ def redact_request_xml(
 
         This function employs the following regular expression:
         {xml_key}(.*)>.*<\/{xml_key} - Designed to match the above pattern by starting and ending
-        with the xmltag string as shown, but the starting tage allows for any characters between
+        with the xmltag string as shown, but the starting tag allows for any characters between
         "xmltag" and the > character to allow for the attribute specification shown above. This
         results in the starting of the xml as {xml_key}(.*)> and the ending as <\/{xml_key}.
-        The miscellaneous characters are "captured" as a variable and preserved by the
+        The characters between the xml tags are "captured" as a variable and preserved by the
         substitution operation through the use of the \1 supplied in the replacement string.
         Between these tags, any non-newline characters are allowed using the .* expression.
         """
@@ -246,21 +247,18 @@ def redact_request_xml(
             is_bytes = True
             xml_string = xml_string.decode("utf-8")
         for xml_key in secret_traits.values():
-            match = re.search(rf"\<{xml_key}+[^>]*\>", xml_string)
-            if not match:
-                continue
+            start_tag_eng_tag_regex = rf"{xml_key}(.*)>.*<\/{xml_key}"
+            redacted_regex = rf"{xml_key}\1>********</{xml_key}"
             # Delete operation has no end tag and and redaction should not be attempted.
             #
             # Redact this:
             # <tag operation="set">secret</tag>
             #
             # Don't try to redact this:
             # <tag operation="del" />
-            if f"</{xml_key}>" not in xml_string:
-                continue
             xml_string = re.sub(
-                rf"{xml_key}(.*)>.*<\/{xml_key}",
-                rf"{xml_key}\1>********</{xml_key}",
+                start_tag_eng_tag_regex,
+                redacted_regex,
                 xml_string,
             )
         if is_bytes:
@@ -271,8 +269,7 @@ def redact_result_xml(
         self,
         security_result: Union[str, bytes, List[int]],
         secret_traits: dict,
-        racf_key_map: dict = {},
-        racf_message_key_map: dict = {},
+        racf_trait_and_message_key_map: dict = {},
     ) -> str:
         """
         Redacts a list of specific secret traits in a result xml string.
@@ -286,32 +283,38 @@ def redact_result_xml(
             return security_result
         for xml_key in secret_traits.values():
             racf_key = xml_key.split(":")[1] if ":" in xml_key else xml_key
-            if racf_key in racf_key_map:
-                print(racf_key_map[racf_key])
-                racf_key = racf_key_map[racf_key]
+            if racf_key in racf_trait_and_message_key_map.get("trait_map", {}):
+                racf_key = racf_trait_and_message_key_map["trait_map"][racf_key]
+            racf_command_argument_regex = rf"{racf_key.upper()}( +){{0,}}\("
             if isinstance(security_result, bytes):
                 match = re.search(
-                    rf"{racf_key.upper()}( +){{0,}}\(", security_result.decode("cp1047")
+                    racf_command_argument_regex, security_result.decode("cp1047")
                 )
             else:
                 match = re.search(rf"{racf_key.upper()}( +){{0,}}\(", security_result)
             if not match:
                 continue
             security_result = self.__redact_string(security_result, racf_key)
-            if racf_key in racf_message_key_map:
-                racf_key = racf_message_key_map[racf_key]
+            if racf_key in racf_trait_and_message_key_map.get("message_map", {}):
+                racf_key = racf_trait_and_message_key_map["message_map"][racf_key]
+            racf_message_regex = (
+                r"<message>([A-Z]*[0-9]*[A-Z]) [^<>]*"
+                + rf"{racf_key.upper()}[^<>]*<\/message>"
+            )
+            redacted_racf_message_regex = (
+                rf"<message>REDACTED MESSAGE CONCERNING {racf_key.upper()}, "
+                + r"REVIEW DOCUMENTATION OF \1 FOR MORE INFORMATION</message>"
+            )
             if isinstance(security_result, bytes):
                 security_result = re.sub(
-                    rf"<message>([A-Z]*[0-9]*[A-Z]) [^<>]*{racf_key.upper()}[^<>]*<\/message>",
-                    rf"<message>REDACTED MESSAGE CONCERNING {racf_key.upper()}, "
-                    + r"REVIEW DOCUMENTATION OF \1 FOR MORE INFORMATION</message>",
+                    racf_message_regex,
+                    redacted_racf_message_regex,
                     security_result.decode("cp1047"),
                 ).encode("cp1047")
             else:
                 security_result = re.sub(
-                    rf"<message>([A-Z]*[0-9]*[A-Z]) [^<>]*{racf_key.upper()}[^<>]*<\/message>",
-                    rf"<message>REDACTED MESSAGE CONCERNING {racf_key.upper()}, "
-                    + r"REVIEW DOCUMENTATION OF \1 FOR MORE INFORMATION</message>",
+                    racf_message_regex,
+                    redacted_racf_message_regex,
                     security_result,
                 )
         return security_result
@@ -449,8 +452,7 @@ def log_hex_dump(
         self,
         raw_result_xml: bytes,
         secret_traits: dict,
-        racf_key_map: dict,
-        racf_message_key_map: dict,
+        racf_trait_and_message_key_map: dict,
     ) -> None:
         """
         Log the raw result XML returned by IRRSMO00 as a hex dump.
@@ -464,8 +466,7 @@ def log_hex_dump(
         raw_result_xml = self.redact_result_xml(
             raw_result_xml,
             secret_traits,
-            racf_key_map,
-            racf_message_key_map,
+            racf_trait_and_message_key_map,
         )
         for byte in raw_result_xml:
             color_function = self.__green

pyracf/data_set/data_set_admin.py

@@ -63,14 +63,16 @@ def __init__(
             "dfp": {"dfp:owner": "racf:resowner", "dfp:ckds_data_key": "racf:datakey"},
             "tme": {"tme:roles": "racf:roles"},
         }
-        self._racf_key_map = {
+        self._racf_trait_and_message_key_map["trait_map"] = {
             "audaltr": "audit",
             "audcntl": "audit",
             "audnone": "audit",
             "audread": "audit",
             "audupdt": "audit",
         }
-        self._racf_message_key_map = {"uacc": "universal access"}
+        self._racf_trait_and_message_key_map["message_map"] = {
+            "uacc": "universal access"
+        }
         self._valid_segment_traits["base"].update(
             self._common_base_traits_data_set_generic
         )

pyracf/resource/resource_admin.py

@@ -238,14 +238,16 @@ def __init__(
                 "sigrequired": "signatureRequired",
             },
         }
-        self._racf_key_map = {
+        self._racf_trait_and_message_key_map["trait_map"] = {
             "audaltr": "audit",
             "audcntl": "audit",
             "audnone": "audit",
             "audread": "audit",
             "audupdt": "audit",
         }
-        self._racf_message_key_map = {"uacc": "universal access"}
+        self._racf_trait_and_message_key_map["message_map"] = {
+            "uacc": "universal access"
+        }
         super().__init__(
             "resource",
             irrsmo00_result_buffer_size=irrsmo00_result_buffer_size,