INTEG 1001 - Risk Tag Error handling (#57)

Author: Juliya Smith

CHANGELOG.md

@@ -16,6 +16,12 @@ how a consumer would use the library (e.g. adding unit tests, updating documenta
 - Additional information in the error log file:
     - The full command path for the command that errored.
     - User-facing error messages you see during adhoc sessions.
+- A custom error in the error log when you try adding unknown risk tags to user.
+- A custom error in the error log when you try adding a user to a detection list who is already added.
+
+### Fixed
+
+- Fixed bug in bulk commands where value-less fields in csv files were treated as empty strings instead of None.
 
 ### 0.5.3 - 2020-05-04
 

src/code42cli/cmds/detectionlists/__init__.py

@@ -1,3 +1,5 @@
+from py42.exceptions import Py42BadRequestError
+
 from code42cli.compat import str
 from code42cli.cmds.detectionlists.commands import DetectionListCommandFactory
 from code42cli.bulk import generate_template, run_bulk_process, CSVReader, FlatFileReader
@@ -6,6 +8,7 @@
     BulkCommandType,
     DetectionLists,
     DetectionListUserKeys,
+    RiskTags,
 )
 
 
@@ -15,7 +18,15 @@ def __init__(self, username, list_name):
         super(UserAlreadyAddedError, self).__init__(msg)
 
 
-def handle_bad_request_during_add(bad_request_err, username_tried_adding, list_name):
+class UnknownRiskTagError(Exception):
+    def __init__(self, bad_tags):
+        tags = u", ".join(bad_tags)
+        super(UnknownRiskTagError, self).__init__(
+            u"The following risk tags are unknown: '{}'.".format(tags)
+        )
+
+
+def try_handle_user_already_added_error(bad_request_err, username_tried_adding, list_name):
     if _error_is_user_already_added(bad_request_err.response.text):
         raise UserAlreadyAddedError(username_tried_adding, list_name)
     return False
@@ -209,6 +220,7 @@ def update_user(sdk, user_id, cloud_alias=None, risk_tag=None, notes=None):
     """Updates a detection list user.
     
     Args:
+        sdk (py42.sdk.SDKClient): py42 sdk.
         user_id (str or unicode): The ID of the user to update. This is their `userUid` found from 
             `sdk.users.get_by_username()`.
         cloud_alias (str or unicode): A cloud alias to add to the user.
@@ -218,6 +230,29 @@ def update_user(sdk, user_id, cloud_alias=None, risk_tag=None, notes=None):
     if cloud_alias:
         sdk.detectionlists.add_user_cloud_alias(user_id, cloud_alias)
     if risk_tag:
-        sdk.detectionlists.add_user_risk_tags(user_id, risk_tag)
+        try_add_risk_tags(sdk, user_id, risk_tag)
     if notes:
         sdk.detectionlists.update_user_notes(user_id, notes)
+
+
+def try_add_risk_tags(sdk, user_id, risk_tag):
+    _try_add_or_remove_risk_tags(user_id, risk_tag, sdk.detectionlists.add_user_risk_tags)
+
+
+def try_remove_risk_tags(sdk, user_id, risk_tag):
+    _try_add_or_remove_risk_tags(user_id, risk_tag, sdk.detectionlists.remove_user_risk_tags)
+
+
+def _try_add_or_remove_risk_tags(user_id, risk_tag, func):
+    try:
+        func(user_id, risk_tag)
+    except Py42BadRequestError:
+        _try_handle_bad_risk_tag(risk_tag)
+        raise
+
+
+def _try_handle_bad_risk_tag(tags):
+    options = list(RiskTags())
+    unknowns = [tag for tag in tags if tag not in options] if tags else None
+    if unknowns:
+        raise UnknownRiskTagError(unknowns)

src/code42cli/cmds/detectionlists/departing_employee.py

@@ -4,7 +4,7 @@
     load_user_descriptions,
     get_user_id,
     update_user,
-    handle_bad_request_during_add,
+    try_handle_user_already_added_error,
 )
 from code42cli.cmds.detectionlists.enums import DetectionLists
 
@@ -42,8 +42,9 @@ def add_departing_employee(
         sdk.detectionlists.departing_employee.add(user_id, departure_date)
         update_user(sdk, user_id, cloud_alias, notes=notes)
     except Py42BadRequestError as err:
-        if not handle_bad_request_during_add(err, username, DetectionLists.DEPARTING_EMPLOYEE):
-            raise
+        list_name = DetectionLists.DEPARTING_EMPLOYEE
+        try_handle_user_already_added_error(err, username, list_name)
+        raise
 
 
 def remove_departing_employee(sdk, profile, username):

src/code42cli/cmds/detectionlists/enums.py

@@ -16,3 +16,26 @@ class DetectionListUserKeys(object):
     USERNAME = u"username"
     NOTES = u"notes"
     RISK_TAG = u"risk_tag"
+
+
+class RiskTags(object):
+    FLIGHT_RISK = u"FLIGHT_RISK"
+    HIGH_IMPACT_EMPLOYEE = u"HIGH_IMPACT_EMPLOYEE"
+    ELEVATED_ACCESS_PRIVILEGES = u"ELEVATED_ACCESS_PRIVILEGES"
+    PERFORMANCE_CONCERNS = u"PERFORMANCE_CONCERNS"
+    SUSPICIOUS_SYSTEM_ACTIVITY = u"SUSPICIOUS_SYSTEM_ACTIVITY"
+    POOR_SECURITY_PRACTICES = u"POOR_SECURITY_PRACTICES"
+    CONTRACT_EMPLOYEE = u"CONTRACT_EMPLOYEE"
+
+    def __iter__(self):
+        return iter(
+            [
+                self.FLIGHT_RISK,
+                self.HIGH_IMPACT_EMPLOYEE,
+                self.ELEVATED_ACCESS_PRIVILEGES,
+                self.PERFORMANCE_CONCERNS,
+                self.SUSPICIOUS_SYSTEM_ACTIVITY,
+                self.POOR_SECURITY_PRACTICES,
+                self.CONTRACT_EMPLOYEE,
+            ]
+        )

src/code42cli/cmds/detectionlists/high_risk_employee.py

@@ -5,17 +5,17 @@
     load_username_description,
     get_user_id,
     update_user,
-    handle_bad_request_during_add,
+    try_handle_user_already_added_error,
+    try_add_risk_tags,
+    try_remove_risk_tags,
 )
-from code42cli.cmds.detectionlists.enums import DetectionLists
-from code42cli.cmds.detectionlists.enums import DetectionListUserKeys
+from code42cli.cmds.detectionlists.enums import DetectionLists, DetectionListUserKeys, RiskTags
 from code42cli.commands import Command
 
 from py42.exceptions import Py42BadRequestError
 
 
 def load_subcommands():
-
     handlers = _create_handlers()
     detection_list = DetectionList.create_high_risk_employee_list(handlers)
     cmd_list = detection_list.load_subcommands()
@@ -47,13 +47,13 @@ def _create_handlers():
 def add_risk_tags(sdk, profile, username, risk_tag):
     risk_tag = _handle_list_args(risk_tag)
     user_id = get_user_id(sdk, username)
-    sdk.detectionlists.add_user_risk_tags(user_id, risk_tag)
+    try_add_risk_tags(sdk, user_id, risk_tag)
 
 
 def remove_risk_tags(sdk, profile, username, risk_tag):
     risk_tag = _handle_list_args(risk_tag)
     user_id = get_user_id(sdk, username)
-    sdk.detectionlists.remove_user_risk_tags(user_id, risk_tag)
+    try_remove_risk_tags(sdk, user_id, risk_tag)
 
 
 def add_high_risk_employee(sdk, profile, username, cloud_alias=None, risk_tag=None, notes=None):
@@ -74,8 +74,9 @@ def add_high_risk_employee(sdk, profile, username, cloud_alias=None, risk_tag=No
         sdk.detectionlists.high_risk_employee.add(user_id)
         update_user(sdk, user_id, cloud_alias, risk_tag, notes)
     except Py42BadRequestError as err:
-        if not handle_bad_request_during_add(err, username, DetectionLists.HIGH_RISK_EMPLOYEE):
-            raise
+        list_name = DetectionLists.HIGH_RISK_EMPLOYEE
+        try_handle_user_already_added_error(err, username, list_name)
+        raise
 
 
 def remove_high_risk_employee(sdk, profile, username):
@@ -93,16 +94,9 @@ def remove_high_risk_employee(sdk, profile, username):
 def _load_risk_tag_description(argument_collection):
     risk_tag = argument_collection.arg_configs[DetectionListUserKeys.RISK_TAG]
     risk_tag.as_multi_val_param()
+    tags = u", ".join(list(RiskTags()))
     risk_tag.set_help(
-        u"Risk tags associated with the employee. "
-        u"Options include "
-        u"[HIGH_IMPACT_EMPLOYEE, "
-        u"ELEVATED_ACCESS_PRIVILEGES, "
-        u"PERFORMANCE_CONCERNS, "
-        u"FLIGHT_RISK, "
-        u"SUSPICIOUS_SYSTEM_ACTIVITY, "
-        u"POOR_SECURITY_PRACTICES, "
-        u"CONTRACT_EMPLOYEE]"
+        u"Risk tags associated with the employee. Options include: [{}].".format(tags)
     )
 
 

tests/cmds/detectionlists/conftest.py

@@ -27,9 +27,9 @@ def bad_request_for_user_already_added(mocker):
 
 
 @pytest.fixture
-def bad_request_for_other_reasons(mocker):
+def generic_bad_request(mocker):
     resp = mocker.MagicMock(spec=Response)
-    resp.text = "Some other, non-supported reason for a bad request"
+    resp.text = "TEST"
     return _create_bad_request_mock(resp)
 
 

tests/cmds/detectionlists/test_departing_employee.py

@@ -60,9 +60,9 @@ def test_add_departing_employee_when_user_already_added_raises_UserAlreadyAddedE
 
 
 def test_add_departing_employee_when_bad_request_but_not_user_already_added_raises_Py42BadRequestError(
-    sdk_with_user, profile, bad_request_for_other_reasons, caplog
+    sdk_with_user, profile, generic_bad_request, caplog
 ):
-    sdk_with_user.detectionlists.departing_employee.add.side_effect = bad_request_for_other_reasons
+    sdk_with_user.detectionlists.departing_employee.add.side_effect = generic_bad_request
     with pytest.raises(Py42BadRequestError):
         add_departing_employee(sdk_with_user, profile, _EMPLOYEE)