Bugfix: make profile name required for delete (#175)

Author: Juliya Smith

CHANGELOG.md

@@ -10,6 +10,11 @@ how a consumer would use the library (e.g. adding unit tests, updating documenta
 
 ## Unreleased
 
+### Fixed
+
+- Issue where `code42 profile delete` was allowed without giving a `profile_name` even
+  though deleting the default profile is not allowed.
+
 ### Added
 
 - `code42 audit-logs` commands:
@@ -18,6 +23,8 @@ how a consumer would use the library (e.g. adding unit tests, updating documenta
 
 ### Changed
 
+- `profile_name` argument is now required for `code42 profile delete`, as it was meant to be.
+
 - The `--advanced-query` option on `alerts search` and `security-data (search|send-to)` commands has been updated:
     - It can now accept the query as a JSON string or as the path to a file containing the JSON query.
     - It can be used with the `--use-checkpoint/-c` option.

src/code42cli/cmds/profile.py

@@ -18,7 +18,10 @@ def profile():
     pass
 
 
-profile_name_arg = click.argument("profile_name", required=False)
+def profile_name_arg(required=False):
+    return click.argument("profile_name", required=required)
+
+
 name_option = click.option(
     "-n",
     "--name",
@@ -48,7 +51,7 @@ def profile():
 
 
 @profile.command()
-@profile_name_arg
+@profile_name_arg()
 def show(profile_name):
     """Print the details of a profile."""
     c42profile = cliprofile.get_profile(profile_name)
@@ -96,7 +99,7 @@ def update(name, server, username, password, disable_ssl_errors):
 
 
 @profile.command()
-@profile_name_arg
+@profile_name_arg()
 def reset_pw(profile_name):
     """\b
     Change the stored password for a profile. Only affects what's stored in the local profile,
@@ -117,7 +120,7 @@ def _list():
 
 
 @profile.command()
-@profile_name_arg
+@profile_name_arg()
 def use(profile_name):
     """Set a profile as the default."""
     cliprofile.switch_default_profile(profile_name)
@@ -126,7 +129,7 @@ def use(profile_name):
 
 @profile.command()
 @yes_option
-@profile_name_arg
+@profile_name_arg(required=True)
 def delete(profile_name):
     """Deletes a profile and its stored password (if any)."""
     message = "\nDeleting this profile will also delete any stored passwords and checkpoints. Are you sure? (y/n): "

src/code42cli/profile.py

@@ -108,6 +108,7 @@ def create_profile(name, server, username, ignore_ssl_errors):
 
 def delete_profile(profile_name):
     profile = _get_profile(profile_name)
+    profile_name = profile.name
     if password.get_stored_password(profile) is not None:
         password.delete_password(profile)
     cursor_stores = get_all_cursor_stores_for_profile(profile_name)

tests/cmds/test_profile.py

@@ -333,6 +333,12 @@ def test_delete_profile_warns_if_deleting_default(runner, mock_cliprofile_namesp
     assert "'mockdefault' is currently the default profile!" in result.output
 
 
+def test_delete_profile_requires_profile_name_arg(runner, mock_cliprofile_namespace):
+    result = runner.invoke(cli, ["profile", "delete"])
+    assert "Error: Missing argument 'PROFILE_NAME'." in result.output
+    assert mock_cliprofile_namespace.delete_profile.call_count == 0
+
+
 def test_delete_profile_does_nothing_if_user_doesnt_agree(
     runner, user_disagreement, mock_cliprofile_namespace
 ):

tests/test_profile.py

@@ -201,6 +201,24 @@ def test_set_password_uses_expected_password(config_accessor, password_setter):
     assert password_setter.call_args[0][1] == "newpassword"
 
 
+def test_delete_profile_deletes_profile(config_accessor, mocker):
+    name = "deleteme"
+    profile = create_mock_profile(name)
+    mock_get_profile = mocker.patch("code42cli.profile._get_profile")
+    mock_get_profile.return_value = profile
+    cliprofile.delete_profile(name)
+    config_accessor.delete_profile.assert_called_once_with(name)
+
+
+def test_delete_profile_deletes_profile_from_object_name(config_accessor, mocker):
+    expected = "deleteme - different name than the arg"
+    profile = create_mock_profile(expected)
+    mock_get_profile = mocker.patch("code42cli.profile._get_profile")
+    mock_get_profile.return_value = profile
+    cliprofile.delete_profile("deleteme")
+    config_accessor.delete_profile.assert_called_once_with(expected)
+
+
 def test_delete_profile_deletes_password_if_exists(
     config_accessor, mocker, password_getter, password_deleter
 ):