Feature/profile deletion (#30)

* -Add validation logic around default profile.
-Add help message for when default is not set (or points to an invalid/missing entry).

* - update base cursor store to allow row deletions.
- add clean() method to FileEventCursorStore to remove a profile's row(s).

* add delete_password() to password module

* add delete_profile() to ConfigAccessor

* handle resetting default_profile internal value to __DEFAULT__ when the default profile is deleted.

* add delete_profile() method to profile module that clears config/password/cursor_store

* - add `delete_profile()`, `delete_all_profiles()` functions
- add `delete`, and `delete_all` Commands and register them
- fix typo in `_load_profile_create_descriptions()`

* merge error

* fix more merge errors

* fix the merge error fix error

* address PR feedback

* update changelog

* remove unused DEFAULT_PROFILE_IS_COMPLETE config item.

* attempting some tests

* a working delete_profile_clears_checkpoint test

* fix test for py3.5 compat

* formatting

Author: timabrmsn

CHANGELOG.md

@@ -18,11 +18,14 @@ how a consumer would use the library (e.g. adding unit tests, updating documenta
     - `--filename` flag renamed to `--file-name`.
     - `--filepath` flag renamed to `--file-path`.
     - `--processOwner` flag renamed to `--process-owner`
+- Default profile validation logic added to prevent confusing error states.
 
 ### Added
 
 - `code42 profile update` command.
 - `code42 profile create` command.
+- `code42 profile delete` command.
+- `code42 profile delete-all` command.
 - `code42 detection-lists high-risk` commands:
     - `bulk` with subcommands:
         - `add`: that takes a csv file of users.

src/code42cli/cmds/profile.py

@@ -59,7 +59,21 @@ def load_subcommands():
         arg_customizer=_load_profile_update_descriptions,
     )
 
-    return [show, list_all, use, reset_pw, create, update]
+    delete = Command(
+        u"delete",
+        "Deletes a profile and its stored password (if any).",
+        u"{} {}".format(usage_prefix, u"delete <profile-name>"),
+        handler=delete_profile,
+    )
+
+    delete_all = Command(
+        u"delete-all",
+        u"Deletes all profiles and saved passwords (if any).",
+        u"{} {}".format(usage_prefix, u"delete-all"),
+        handler=delete_all_profiles,
+    )
+
+    return [show, list_all, use, reset_pw, create, update, delete, delete_all]
 
 
 def show_profile(name=None):
@@ -117,6 +131,31 @@ def use_profile(profile):
     cliprofile.switch_default_profile(profile)
 
 
+def delete_profile(name):
+    if cliprofile.is_default_profile(name):
+        print(u"\n{} is currently the default profile!".format(name))
+    if not does_user_agree(
+        u"\nDeleting this profile will also delete any stored passwords and checkpoints. Are you sure? (y/n): "
+    ):
+        return
+    cliprofile.delete_profile(name)
+
+
+def delete_all_profiles():
+    existing_profiles = cliprofile.get_all_profiles()
+    if existing_profiles:
+        print(u"\nAre you sure you want to delete the following profiles?")
+        for profile in existing_profiles:
+            print(u"\t{}".format(profile.name))
+        if does_user_agree(
+            u"\nThis will also delete any stored passwords and checkpoints. (y/n): "
+        ):
+            for profile in existing_profiles:
+                cliprofile.delete_profile(profile.name)
+    else:
+        print(u"\nNo profiles exist. Nothing to delete.")
+
+
 def _load_optional_profile_description(argument_collection):
     profile = argument_collection.arg_configs[u"name"]
     profile.add_short_option_name(u"-n")

src/code42cli/cmds/shared/cursor_store.py

@@ -33,6 +33,13 @@ def _set(self, column_name, new_value, primary_key):
         with self._connection as conn:
             conn.execute(query, (new_value, primary_key))
 
+    def _delete(self, primary_key):
+        query = u"DELETE FROM {0} WHERE {1}=?".format(
+            self._table_name, self._PRIMARY_KEY_COLUMN_NAME
+        )
+        with self._connection as conn:
+            conn.execute(query, (primary_key,))
+
     def _row_exists(self, primary_key):
         query = u"SELECT * FROM {0} WHERE {1}=?"
         query = query.format(self._table_name, self._PRIMARY_KEY_COLUMN_NAME)
@@ -86,6 +93,10 @@ def replace_stored_insertion_timestamp(self, new_insertion_timestamp):
             primary_key=self._primary_key,
         )
 
+    def clean(self):
+        """Removes profile cursor data from store."""
+        self._delete(self._primary_key)
+
     def _init_table(self):
         columns = u"{0}, {1}".format(self._PRIMARY_KEY_COLUMN_NAME, _INSERTION_TIMESTAMP_FIELD_NAME)
         create_table_query = u"CREATE TABLE {0} ({1})".format(self._table_name, columns)
@@ -96,3 +107,7 @@ def _insert_new_row(self):
         insert_query = u"INSERT INTO {0} VALUES(?, null)".format(self._table_name)
         with self._connection as conn:
             conn.execute(insert_query, (self._primary_key,))
+
+
+def get_file_event_cursor_store(profile_name):
+    return FileEventCursorStore(profile_name)

src/code42cli/config.py

@@ -18,7 +18,6 @@ class ConfigAccessor(object):
     AUTHORITY_KEY = u"c42_authority_url"
     USERNAME_KEY = u"c42_username"
     IGNORE_SSL_ERRORS_KEY = u"ignore-ssl-errors"
-    DEFAULT_PROFILE_IS_COMPLETE = u"default_profile_is_complete"
     DEFAULT_PROFILE = u"default_profile"
     _INTERNAL_SECTION = u"Internal"
 
@@ -81,6 +80,15 @@ def switch_default_profile(self, new_default_name):
         self._save()
         print(u"{} has been set as the default profile.".format(new_default_name))
 
+    def delete_profile(self, name):
+        """Deletes a profile."""
+        if self.get_profile(name) is None:
+            raise NoConfigProfileError()
+        self.parser.remove_section(name)
+        if name == self._default_profile_name:
+            self._internal[self.DEFAULT_PROFILE] = self.DEFAULT_VALUE
+        self._save()
+
     def _set_authority_url(self, new_value, profile):
         profile[self.AUTHORITY_KEY] = new_value.strip()
 
@@ -112,7 +120,6 @@ def _get_profile_names(self):
     def _create_internal_section(self):
         self.parser.add_section(self._INTERNAL_SECTION)
         self.parser[self._INTERNAL_SECTION] = {}
-        self.parser[self._INTERNAL_SECTION][self.DEFAULT_PROFILE_IS_COMPLETE] = str(False)
         self.parser[self._INTERNAL_SECTION][self.DEFAULT_PROFILE] = self.DEFAULT_VALUE
 
     def _create_profile_section(self, name):

src/code42cli/password.py

@@ -29,6 +29,12 @@ def set_password(profile, new_password):
     keyring.set_password(service_name, profile.username, new_password)
 
 
+def delete_password(profile):
+    """Deletes password for the given profile name."""
+    service_name = _get_keyring_service_name(profile.name)
+    keyring.delete_password(service_name, profile.username)
+
+
 def _get_keyring_service_name(profile_name):
     return u"{}::{}".format(PRODUCT_NAME, profile_name)
 

src/code42cli/profile.py

@@ -1,4 +1,5 @@
 import code42cli.password as password
+from code42cli.cmds.shared.cursor_store import get_file_event_cursor_store
 from code42cli.config import ConfigAccessor, config_accessor, NoConfigProfileError
 from code42cli.util import (
     print_error,
@@ -70,6 +71,12 @@ def default_profile_exists():
         return False
 
 
+def is_default_profile(name):
+    if default_profile_exists():
+        default = get_profile()
+        return name == default.name
+
+
 def validate_default_profile():
     if not default_profile_exists():
         existing_profiles = get_all_profiles()
@@ -100,6 +107,15 @@ def create_profile(name, server, username, ignore_ssl_errors):
     config_accessor.create_profile(name, server, username, ignore_ssl_errors)
 
 
+def delete_profile(profile_name):
+    profile = _get_profile(profile_name)
+    if password.get_stored_password(profile) is not None:
+        password.delete_password(profile)
+    cursor_store = get_file_event_cursor_store(profile_name)
+    cursor_store.clean()
+    config_accessor.delete_profile(profile_name)
+
+
 def update_profile(name, server, username, ignore_ssl_errors):
     config_accessor.update_profile(name, server, username, ignore_ssl_errors)
 

tests/cmds/securitydata/test_cursor_store.py

@@ -79,3 +79,15 @@ def test_replace_stored_insertion_timestamp_executes_query_with_expected_primary
         with store._connection as conn:
             actual = conn.execute.call_args[0][1][0]
             assert actual == new_insertion_timestamp
+
+    def test_clean_executes_query_with_expected_primary_key(self, sqlite_connection):
+        profile_name = "Profile"
+        store = FileEventCursorStore(profile_name, self.MOCK_TEST_DB_NAME)
+        store.clean()
+        with store._connection as conn:
+            expected_query = "DELETE FROM {0} WHERE {1}=?".format(
+                store._table_name, store._PRIMARY_KEY_COLUMN_NAME
+            )
+            actual_query, pk = conn.execute.call_args[0]
+            assert expected_query == actual_query
+            assert pk == (profile_name,)

tests/cmds/test_profile.py

@@ -163,6 +163,51 @@ def test_update_profile_if_user_agrees_and_valid_connection_sets_password(
     mock_cliprofile_namespace.set_password.assert_called_once_with("newpassword", mocker.ANY)
 
 
+def test_delete_profile_warns_if_deleting_default(
+    capsys, user_agreement, mock_cliprofile_namespace
+):
+    mock_cliprofile_namespace.is_default_profile.return_value = True
+    profilecmd.delete_profile("mockdefault")
+    capture = capsys.readouterr()
+    assert "mockdefault is currently the default profile!" in capture.out
+    
+
+def test_delete_all_warns_if_profiles_exist(capsys, user_agreement, mock_cliprofile_namespace):
+    mock_cliprofile_namespace.get_all_profiles.return_value = [
+        create_mock_profile("test1"),
+        create_mock_profile("test2"),
+    ]
+    profilecmd.delete_all_profiles()
+    capture = capsys.readouterr()
+    assert "Are you sure you want to delete the following profiles?" in capture.out
+    assert "test1" in capture.out
+    assert "test2" in capture.out
+    
+
+def test_delete_profile_does_nothing_if_user_doesnt_agree(
+    user_disagreement, mock_cliprofile_namespace
+):
+    profilecmd.delete_profile("mockprofile")
+    assert mock_cliprofile_namespace.delete_profile.call_count == 0 
+    
+    
+def test_delete_all_profiles_does_nothing_if_user_doesnt_agree(
+    user_disagreement, mock_cliprofile_namespace
+):
+    profilecmd.delete_all_profiles()
+    assert mock_cliprofile_namespace.delete_profile.call_count == 0
+    
+    
+def test_delete_all_deletes_all_existing_profiles(user_agreement, mock_cliprofile_namespace):
+    mock_cliprofile_namespace.get_all_profiles.return_value = [
+        create_mock_profile("test1"),
+        create_mock_profile("test2"),
+    ]
+    profilecmd.delete_all_profiles()
+    mock_cliprofile_namespace.delete_profile.assert_any_call("test1")
+    mock_cliprofile_namespace.delete_profile.assert_any_call("test2")
+    
+    
 def test_prompt_for_password_reset_if_credentials_valid_password_saved(
     mocker, user_agreement, mock_verify, mock_cliprofile_namespace
 ):

tests/test_config.py

@@ -71,7 +71,6 @@ def create_internal_object(is_complete, default_profile_name=None):
     default_profile_name = default_profile_name or ConfigAccessor.DEFAULT_VALUE
     internal_dict = {
         ConfigAccessor.DEFAULT_PROFILE: default_profile_name,
-        ConfigAccessor.DEFAULT_PROFILE_IS_COMPLETE: is_complete,
     }
     internal_section = MockSection(_INTERNAL, internal_dict)
 
@@ -164,7 +163,6 @@ def test_create_profile_when_no_default_profile_sets_default(
     ):
         mock_profile = create_mock_profile_object(_TEST_PROFILE_NAME, None, None)
         mock_internal = create_internal_object(False)
-        mock_internal["default_profile_is_complete"] = "False"
         setup_parser_one_profile(mock_internal, mock_internal, config_parser_for_create)
         accessor = ConfigAccessor(config_parser_for_create)
         accessor.switch_default_profile = mocker.MagicMock()
@@ -187,7 +185,6 @@ def test_create_profile_when_has_default_profile_does_not_set_default(
     def test_create_profile_when_not_existing_saves(self, config_parser_for_create, mock_saver):
         create_mock_profile_object(_TEST_PROFILE_NAME, None, None)
         mock_internal = create_internal_object(False)
-        mock_internal["default_profile_is_complete"] = "False"
         setup_parser_one_profile(mock_internal, mock_internal, config_parser_for_create)
         accessor = ConfigAccessor(config_parser_for_create)
 
@@ -198,7 +195,6 @@ def test_create_profile_when_not_existing_outputs_confirmation(
         self, capsys, config_parser_for_create, mock_saver
     ):
         mock_internal = create_internal_object(False)
-        mock_internal["default_profile_is_complete"] = "False"
         setup_parser_one_profile(mock_internal, mock_internal, config_parser_for_create)
         accessor = ConfigAccessor(config_parser_for_create)
 

tests/test_profile.py

@@ -3,6 +3,7 @@
 from code42cli import PRODUCT_NAME
 import code42cli.profile as cliprofile
 from code42cli.config import ConfigAccessor, NoConfigProfileError
+from code42cli.cmds.shared.cursor_store import FileEventCursorStore
 from .conftest import MockSection, create_mock_profile
 
 
@@ -23,6 +24,11 @@ def password_getter(mocker):
     return mocker.patch("{}.password.get_stored_password".format(PRODUCT_NAME))
 
 
+@pytest.fixture
+def password_deleter(mocker):
+    return mocker.patch("code42cli.password.delete_password")
+
+
 class TestCode42Profile(object):
     def test_get_password_when_is_none_returns_password_from_getpass(self, mocker, password_getter):
         password_getter.return_value = None
@@ -182,3 +188,25 @@ def test_set_password_uses_expected_password(config_accessor, password_setter):
     test_profile = "testprofilename"
     cliprofile.set_password("newpassword", test_profile)
     assert password_setter.call_args[0][1] == "newpassword"
+
+
+def test_delete_profile_deletes_password_if_exists(
+    config_accessor, mocker, password_getter, password_deleter
+):
+    profile = create_mock_profile("deleteme")
+    mock_get_profile = mocker.patch("code42cli.profile._get_profile")
+    mock_get_profile.return_value = profile
+    password_getter.return_value = "i_exist"
+    cliprofile.delete_profile("deleteme")
+    password_deleter.assert_called_once_with(profile)
+
+
+def test_delete_profile_clears_checkpoint(config_accessor, mocker):
+    profile = create_mock_profile("deleteme")
+    mock_get_profile = mocker.patch("code42cli.profile._get_profile")
+    mock_get_profile.return_value = profile
+    store = mocker.MagicMock(spec=FileEventCursorStore)
+    mock_get_cursor_store = mocker.patch("code42cli.profile.get_file_event_cursor_store")
+    mock_get_cursor_store.return_value = store
+    cliprofile.delete_profile("deleteme")
+    assert store.clean.call_count == 1