Chore/remove error handling (#132)

Author: Juliya Smith

setup.py

@@ -36,7 +36,7 @@
         "c42eventextractor==0.4.0",
         "keyring==18.0.1",
         "keyrings.alt==3.2.0",
-        "py42>=1.7.0",
+        "py42>=1.8.1",
     ],
     extras_require={
         "dev": [

src/code42cli/cmds/alert_rules.py

@@ -2,15 +2,13 @@
 
 import click
 from click import echo
-from py42.exceptions import Py42InternalServerError
 from py42.util import format_json
 
 from code42cli import PRODUCT_NAME
 from code42cli.bulk import generate_template_cmd_factory
 from code42cli.bulk import run_bulk_process
 from code42cli.cmds.shared import get_user_id
 from code42cli.errors import Code42CLIError
-from code42cli.errors import InvalidRuleTypeError
 from code42cli.file_readers import read_csv_arg
 from code42cli.options import format_option
 from code42cli.options import OrderedGroup
@@ -152,23 +150,15 @@ def handle_row(rule_id, username):
 def _add_user(sdk, rule_id, username):
     user_id = get_user_id(sdk, username)
     rules = _get_rule_metadata(sdk, rule_id)
-    try:
-        if rules:
-            sdk.alerts.rules.add_user(rule_id, user_id)
-    except Py42InternalServerError:
-        _check_if_system_rule(rules)
-        raise
+    if rules:
+        sdk.alerts.rules.add_user(rule_id, user_id)
 
 
 def _remove_user(sdk, rule_id, username):
     user_id = get_user_id(sdk, username)
     rules = _get_rule_metadata(sdk, rule_id)
-    try:
-        if rules:
-            sdk.alerts.rules.remove_user(rule_id, user_id)
-    except Py42InternalServerError:
-        _check_if_system_rule(rules)
-        raise
+    if rules:
+        sdk.alerts.rules.remove_user(rule_id, user_id)
 
 
 def _get_all_rules_metadata(sdk):
@@ -185,18 +175,13 @@ def _get_rule_metadata(sdk, rule_id):
 
 
 def _handle_rules_results(rules, rule_id=None):
-    id_msg = "with RuleId {} ".format(rule_id) if rule_id else ""
-    msg = "No alert rules {}found.".format(id_msg)
     if not rules:
+        id_msg = "with RuleId {} ".format(rule_id) if rule_id else ""
+        msg = "No alert rules {}found.".format(id_msg)
         echo(msg)
     return rules
 
 
-def _check_if_system_rule(rules):
-    if rules and rules[0]["isSystem"]:
-        raise InvalidRuleTypeError(rules[0]["observerRuleId"], rules[0]["ruleSource"])
-
-
 def _get_rule_type_func(sdk, rule_type):
     if rule_type == AlertRuleTypes.EXFILTRATION:
         return sdk.alerts.rules.exfiltration.get

src/code42cli/cmds/departing_employee.py

@@ -1,9 +1,7 @@
 import click
-from py42.exceptions import Py42BadRequestError
 
 from code42cli.bulk import generate_template_cmd_factory
 from code42cli.bulk import run_bulk_process
-from code42cli.cmds.detectionlists import try_handle_user_already_added_error
 from code42cli.cmds.detectionlists import update_user
 from code42cli.cmds.detectionlists.options import cloud_alias_option
 from code42cli.cmds.detectionlists.options import notes_option
@@ -124,12 +122,8 @@ def handle_row(username):
 
 def _add_departing_employee(sdk, username, cloud_alias, departure_date, notes):
     user_id = get_user_id(sdk, username)
-    try:
-        sdk.detectionlists.departing_employee.add(user_id, departure_date)
-        update_user(sdk, username, cloud_alias=cloud_alias, notes=notes)
-    except Py42BadRequestError as err:
-        try_handle_user_already_added_error(err, username, "departing-employee list")
-        raise
+    sdk.detectionlists.departing_employee.add(user_id, departure_date)
+    update_user(sdk, username, cloud_alias=cloud_alias, notes=notes)
 
 
 def _remove_departing_employee(sdk, username):

src/code42cli/cmds/detectionlists/__init__.py

@@ -1,5 +1,4 @@
 from code42cli.cmds.shared import get_user_id
-from code42cli.errors import UserAlreadyAddedError
 
 
 def update_user(sdk, username, cloud_alias=None, risk_tag=None, notes=None):
@@ -33,18 +32,6 @@ def remove_risk_tags(sdk, username, risk_tag):
     sdk.detectionlists.remove_user_risk_tags(user_id, risk_tag)
 
 
-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
-
-
-def _error_is_user_already_added(bad_request_error_text):
-    return "User already on list" in bad_request_error_text
-
-
 def handle_list_args(list_arg):
     """Converts str args to a list. Useful for `bulk` commands which don't use click's argument
     parsing but instead pass in values from files, such as in the form "item1 item2"."""

src/code42cli/cmds/high_risk_employee.py

@@ -1,13 +1,11 @@
 import click
 from py42.clients.detectionlists import RiskTags
-from py42.exceptions import Py42BadRequestError
 
 from code42cli.bulk import generate_template_cmd_factory
 from code42cli.bulk import run_bulk_process
 from code42cli.cmds.detectionlists import add_risk_tags as _add_risk_tags
 from code42cli.cmds.detectionlists import handle_list_args
 from code42cli.cmds.detectionlists import remove_risk_tags as _remove_risk_tags
-from code42cli.cmds.detectionlists import try_handle_user_already_added_error
 from code42cli.cmds.detectionlists import update_user
 from code42cli.cmds.detectionlists.options import cloud_alias_option
 from code42cli.cmds.detectionlists.options import notes_option
@@ -174,15 +172,8 @@ def handle_row(username, tag):
 def _add_high_risk_employee(sdk, username, cloud_alias, risk_tag, notes):
     risk_tag = handle_list_args(risk_tag)
     user_id = get_user_id(sdk, username)
-
-    try:
-        sdk.detectionlists.high_risk_employee.add(user_id)
-        update_user(
-            sdk, username, cloud_alias=cloud_alias, risk_tag=risk_tag, notes=notes
-        )
-    except Py42BadRequestError as err:
-        try_handle_user_already_added_error(err, username, "high risk employees list")
-        raise
+    sdk.detectionlists.high_risk_employee.add(user_id)
+    update_user(sdk, username, cloud_alias=cloud_alias, risk_tag=risk_tag, notes=notes)
 
 
 def _remove_high_risk_employee(sdk, username):

src/code42cli/cmds/legal_hold.py

@@ -5,14 +5,10 @@
 
 import click
 from click import echo
-from py42.exceptions import Py42BadRequestError
-from py42.exceptions import Py42ForbiddenError
 
 from code42cli.bulk import generate_template_cmd_factory
 from code42cli.bulk import run_bulk_process
 from code42cli.cmds.shared import get_user_id
-from code42cli.errors import LegalHoldNotFoundOrPermissionDeniedError
-from code42cli.errors import UserAlreadyAddedError
 from code42cli.errors import UserNotInLegalHoldError
 from code42cli.file_readers import read_csv_arg
 from code42cli.options import format_option
@@ -185,16 +181,8 @@ def handle_row(matter_id, username):
 
 def _add_user_to_legal_hold(sdk, matter_id, username):
     user_id = get_user_id(sdk, username)
-    matter = _check_matter_is_accessible(sdk, matter_id)
-    try:
-        sdk.legalhold.add_to_matter(user_id, matter_id)
-    except Py42BadRequestError as e:
-        if "USER_ALREADY_IN_HOLD" in e.response.text:
-            matter_id_and_name_text = "legal hold matter id={}, name={}".format(
-                matter_id, matter["name"]
-            )
-            raise UserAlreadyAddedError(username, matter_id_and_name_text)
-        raise
+    _check_matter_is_accessible(sdk, matter_id)
+    sdk.legalhold.add_to_matter(user_id, matter_id)
 
 
 def _remove_user_from_legal_hold(sdk, matter_id, username):
@@ -255,8 +243,4 @@ def _print_matter_members(username_list, member_type="active"):
 
 @lru_cache(maxsize=None)
 def _check_matter_is_accessible(sdk, matter_id):
-    try:
-        matter = sdk.legalhold.get_matter_by_uid(matter_id)
-        return matter
-    except (Py42BadRequestError, Py42ForbiddenError):
-        raise LegalHoldNotFoundOrPermissionDeniedError(matter_id)
+    return sdk.legalhold.get_matter_by_uid(matter_id)

src/code42cli/cmds/shared.py

@@ -5,8 +5,8 @@
 
 @lru_cache(maxsize=None)
 def get_user_id(sdk, username):
-    """Returns the user's UID (referred to by `user_id` in detection lists). Raises
-    `UserDoesNotExistError` if the user doesn't exist in the Code42 server.
+    """Returns the user's UID (referred to by `user_id` in detection lists).
+    Raises `UserDoesNotExistError` if the user doesn't exist in the Code42 server.
 
     Args:
         sdk (py42.sdk.SDKClient): The py42 sdk.

src/code42cli/errors.py

@@ -5,6 +5,9 @@
 from click._compat import get_text_stderr
 from py42.exceptions import Py42ForbiddenError
 from py42.exceptions import Py42HTTPError
+from py42.exceptions import Py42InvalidRuleOperationError
+from py42.exceptions import Py42LegalHoldNotFoundOrPermissionDeniedError
+from py42.exceptions import Py42UserAlreadyAddedError
 
 from code42cli.logger import get_main_cli_logger
 from code42cli.logger import get_view_error_details_message
@@ -52,19 +55,6 @@ def format_message(self):
         )
 
 
-class UserAlreadyAddedError(Code42CLIError):
-    def __init__(self, username, list_name):
-        msg = "'{}' is already on the {}.".format(username, list_name)
-        super().__init__(msg)
-
-
-class InvalidRuleTypeError(Code42CLIError):
-    def __init__(self, rule_id, source):
-        msg = "Only alert rules with a source of 'Alerting' can be targeted by this command. "
-        msg += "Rule {0} has a source of '{1}'."
-        super().__init__(msg.format(rule_id, source))
-
-
 class UserDoesNotExistError(Code42CLIError):
     """An error to represent a username that is not in our system. The CLI shows this error when
     the user tries to add or remove a user that does not exist. This error is not shown during
@@ -77,20 +67,12 @@ def __init__(self, username):
 class UserNotInLegalHoldError(Code42CLIError):
     def __init__(self, username, matter_id):
         super().__init__(
-            "User '{}' is not an active member of legal hold matter '{}'".format(
+            "User '{}' is not an active member of legal hold matter '{}'.".format(
                 username, matter_id
             )
         )
 
 
-class LegalHoldNotFoundOrPermissionDeniedError(Code42CLIError):
-    def __init__(self, matter_id):
-        super().__init__(
-            "Matter with id={} either does not exist or your profile does not have permission to "
-            "view it.".format(matter_id)
-        )
-
-
 class ExceptionHandlingGroup(click.Group):
     """Custom click.Group subclass to add custom exception handling."""
 
@@ -124,6 +106,15 @@ def invoke(self, ctx):
         except click.exceptions.Exit:
             raise
 
+        except (
+            UserDoesNotExistError,
+            Py42UserAlreadyAddedError,
+            Py42InvalidRuleOperationError,
+            Py42LegalHoldNotFoundOrPermissionDeniedError,
+        ) as err:
+            self.logger.log_error(err)
+            raise Code42CLIError(str(err))
+
         except Py42ForbiddenError as err:
             self.logger.log_verbose_error(self._original_args, err.response.request)
             raise LoggedCLIError(

tests/cmds/conftest.py

@@ -2,16 +2,19 @@
 import threading
 
 import pytest
-from py42.exceptions import Py42BadRequestError
+from py42.exceptions import Py42UserAlreadyAddedError
 from py42.sdk import SDKClient
 from requests import HTTPError
-from requests import Request
 from requests import Response
 from tests.conftest import convert_str_to_date
+from tests.conftest import TEST_ID
 
 from code42cli.logger import CliLogger
 
 
+TEST_EMPLOYEE = "risky employee"
+
+
 @pytest.fixture
 def sdk(mocker):
     return mocker.MagicMock(spec=SDKClient)
@@ -56,26 +59,12 @@ def cli_state_without_user(sdk_without_user, cli_state):
 
 
 @pytest.fixture
-def bad_request_for_user_already_added(mocker):
-    resp = mocker.MagicMock(spec=Response)
-    resp.text = "User already on list"
-    return _create_bad_request_mock(resp)
-
-
-@pytest.fixture
-def generic_bad_request(mocker):
+def user_already_added_error(mocker):
+    err = mocker.MagicMock(spec=HTTPError)
     resp = mocker.MagicMock(spec=Response)
-    req = mocker.MagicMock(spec=Request)
-    req.body = '{"test":"body"}'
-    resp.request = req
-    resp.text = "TEST"
-    return _create_bad_request_mock(resp)
-
-
-def _create_bad_request_mock(resp):
-    base_err = HTTPError()
-    base_err.response = resp
-    return Py42BadRequestError(base_err)
+    resp.text = "TEST_ERR"
+    err.response = resp
+    return Py42UserAlreadyAddedError(err, TEST_ID, "detection list")
 
 
 def get_filter_value_from_json(json, filter_index):