Feature/update profile (#29)

Author: Juliya Smith

CHANGELOG.md

@@ -10,7 +10,7 @@ how a consumer would use the library (e.g. adding unit tests, updating documenta
 
 ## Unreleased
 
-### Changes
+### Changed
 
 - `securitydata` renamed to `security-data`.
 - From `security-data` related subcommands (such as `print`):
@@ -22,6 +22,7 @@ how a consumer would use the library (e.g. adding unit tests, updating documenta
 ### Added
 
 - `code42 profile create` command.
+- `code42 profile update` command.
 
 ### Removed
 

src/code42cli/cmds/profile.py

@@ -48,10 +48,18 @@ def load_subcommands():
         u"Create profile settings. The first profile created will be the default.",
         u"{} {}".format(usage_prefix, u"create <profile-name> <server-address> <username>"),
         handler=create_profile,
-        arg_customizer=_load_profile_create_descripions,
+        arg_customizer=_load_profile_create_descriptions,
     )
 
-    return [show, list_all, use, reset_pw, create]
+    update = Command(
+        u"update",
+        u"Update an existing profile.",
+        u"{} {}".format(usage_prefix, u"update <optional args>"),
+        handler=update_profile,
+        arg_customizer=_load_profile_update_descriptions,
+    )
+
+    return [show, list_all, use, reset_pw, create, update]
 
 
 def show_profile(profile=None):
@@ -67,27 +75,31 @@ def show_profile(profile=None):
 
 
 def create_profile(profile, server, username, disable_ssl_errors=False):
-    """Sets the given profile using command line arguments."""
-    if cliprofile.profile_exists(profile):
-        print_error(u"A profile named {} already exists.".format(profile))
-        exit(1)
-
     cliprofile.create_profile(profile, server, username, disable_ssl_errors)
     _prompt_for_allow_password_set(profile)
 
 
+def update_profile(profile=None, server=None, username=None, disable_ssl_errors=None):
+    profile = cliprofile.get_profile(profile)
+    cliprofile.update_profile(profile.name, server, username, disable_ssl_errors)
+    _prompt_for_allow_password_set(profile.name)
+
+
 def prompt_for_password_reset(profile=None):
     """Securely prompts for your password and then stores it using keyring."""
     c42profile = cliprofile.get_profile(profile)
     new_password = getpass()
+    _validate_connection(c42profile.authority_url, c42profile.username, new_password)
+    cliprofile.set_password(new_password, c42profile.name)
 
-    if not validate_connection(c42profile.authority_url, c42profile.username, new_password):
+
+def _validate_connection(authority, username, password):
+    if not validate_connection(authority, username, password):
         print_error(
             u"Your credentials failed to validate, so your password was not stored."
             u"Check your network connection and the spelling of your username and server URL."
         )
         exit(1)
-    cliprofile.set_password(new_password, c42profile.name)
 
 
 def list_profiles(*args):
@@ -110,13 +122,24 @@ def _load_profile_description(argument_collection):
     profile.set_help(PROFILE_HELP)
 
 
-def _load_profile_create_descripions(argument_collection):
+def _load_profile_create_descriptions(argument_collection):
     profile = argument_collection.arg_configs["profile"]
+    profile.set_help(u"The name to give the profile being created.")
+    _load_profile_settings_descriptions(argument_collection)
+
+
+def _load_profile_update_descriptions(argument_collection):
+    profile = argument_collection.arg_configs["profile"]
+    profile.set_help(u"The name to give the profile being updated.")
+    _load_profile_settings_descriptions(argument_collection)
+    argument_collection.arg_configs["server"].add_short_option_name("-s")
+    argument_collection.arg_configs["username"].add_short_option_name("-u")
+
+
+def _load_profile_settings_descriptions(argument_collection):
     server = argument_collection.arg_configs["server"]
     username = argument_collection.arg_configs["username"]
-
     disable_ssl_errors = argument_collection.arg_configs["disable_ssl_errors"]
-    profile.set_help(u"The name to give the profile being created.")
     server.set_help(u"The url and port of the Code42 server.")
     username.set_help(u"The username of the Code42 API user.")
     disable_ssl_errors.set_help(

src/code42cli/cmds/securitydata/enums.py

@@ -46,6 +46,7 @@ def __iter__(self):
 class SearchArguments(object):
     """These string values should match `argparse` stored parameter names. For example, for the 
     CLI argument `--c42-username`, the string should be `c42_username`."""
+
     ADVANCED_QUERY = u"advanced_query"
     BEGIN_DATE = u"begin"
     END_DATE = u"end"

src/code42cli/commands.py

@@ -22,7 +22,7 @@ class Command(object):
 
         description (str or unicode): Descriptive text to be displayed when using -h.
 
-        usage (str, optional): A usage example to be displayed when using -h.
+        usage (str or unicode, optional): A usage example to be displayed when using -h.
         handler (function, optional): The function to be exectued when the command is run.
 
         arg_customizer (function, optional): A function accepting a single `ArgCollection`

src/code42cli/config.py

@@ -7,6 +7,11 @@
 from code42cli.compat import str
 
 
+class NoConfigProfileError(Exception):
+    def __init__(self):
+        super(Exception, self).__init__(u"Profile does not exist.")
+
+
 class ConfigAccessor(object):
     DEFAULT_VALUE = u"__DEFAULT__"
     AUTHORITY_KEY = u"c42_authority_url"
@@ -31,9 +36,9 @@ def get_profile(self, name=None):
         If the name does not exist or there is no existing profile, it will throw an exception.
         """
         name = name or self._default_profile_name
-        if name not in self.parser.sections() or name == self.DEFAULT_VALUE:
-            raise Exception(u"Profile does not exist.")
-        return self.parser[name]
+        if name not in self._get_sections() or name == self.DEFAULT_VALUE:
+            raise NoConfigProfileError()
+        return self._get_profile(name)
 
     def get_all_profiles(self):
         """Returns all the available profiles."""
@@ -47,21 +52,30 @@ def create_profile(self, name, server, username, ignore_ssl_errors):
         """Creates a new profile if one does not already exist for that name."""
         try:
             self.get_profile(name)
-        except Exception as ex:
+        except NoConfigProfileError as ex:
             if name is not None and name != self.DEFAULT_VALUE:
                 self._create_profile_section(name)
             else:
                 raise ex
-        profile = self.parser[name]
-        self._set_authority_url(server, profile)
-        self._set_username(username, profile)
-        self._set_ignore_ssl_errors(ignore_ssl_errors, profile)
+
+        profile = self.get_profile(name)
+        self.update_profile(profile.name, server, username, ignore_ssl_errors)
         self._try_complete_setup(profile)
 
+    def update_profile(self, name, server=None, username=None, ignore_ssl_errors=None):
+        profile = self.get_profile(name)
+        if server:
+            self._set_authority_url(server, profile)
+        if username:
+            self._set_username(username, profile)
+        if ignore_ssl_errors is not None:
+            self._set_ignore_ssl_errors(ignore_ssl_errors, profile)
+        self._save()
+
     def switch_default_profile(self, new_default_name):
         """Changes what is marked as the default profile in the internal section."""
         if self.get_profile(new_default_name) is None:
-            raise Exception(u"Profile does not exist.")
+            raise NoConfigProfileError()
         self._internal[self.DEFAULT_PROFILE] = new_default_name
         self._save()
         print(u"{} has been set as the default profile.".format(new_default_name))
@@ -75,6 +89,12 @@ def _set_username(self, new_value, profile):
     def _set_ignore_ssl_errors(self, new_value, profile):
         profile[self.IGNORE_SSL_ERRORS_KEY] = str(new_value)
 
+    def _get_sections(self):
+        return self.parser.sections()
+
+    def _get_profile(self, name):
+        return self.parser[name]
+
     @property
     def _internal(self):
         return self.parser[self._INTERNAL_SECTION]
@@ -84,7 +104,7 @@ def _default_profile_name(self):
         return self._internal[self.DEFAULT_PROFILE]
 
     def _get_profile_names(self):
-        names = list(self.parser.sections())
+        names = list(self._get_sections())
         names.remove(self._INTERNAL_SECTION)
         return names
 

src/code42cli/profile.py

@@ -1,5 +1,5 @@
 import code42cli.password as password
-from code42cli.config import ConfigAccessor, config_accessor
+from code42cli.config import ConfigAccessor, config_accessor, NoConfigProfileError
 from code42cli.util import print_error, print_create_profile_help
 
 
@@ -23,6 +23,11 @@ def username(self):
     def ignore_ssl_errors(self):
         return self._profile[ConfigAccessor.IGNORE_SSL_ERRORS_KEY]
 
+    @property
+    def has_stored_password(self):
+        stored_password = password.get_stored_password(self)
+        return stored_password is not None and stored_password != u""
+
     def get_password(self):
         pwd = password.get_stored_password(self)
         if not pwd:
@@ -37,13 +42,14 @@ def __str__(self):
 
 def _get_profile(profile_name=None):
     """Returns the profile for the given name."""
-    return Code42Profile(config_accessor.get_profile(profile_name))
+    config_profile = config_accessor.get_profile(profile_name)
+    return Code42Profile(config_profile)
 
 
 def get_profile(profile_name=None):
     try:
         return _get_profile(profile_name)
-    except Exception as ex:
+    except NoConfigProfileError as ex:
         print_error(str(ex))
         print_create_profile_help()
         exit(1)
@@ -53,15 +59,15 @@ def default_profile_exists():
     try:
         profile = _get_profile()
         return profile.name and profile.name != ConfigAccessor.DEFAULT_VALUE
-    except Exception:
+    except NoConfigProfileError:
         return False
 
 
 def profile_exists(profile_name=None):
     try:
         _get_profile(profile_name)
         return True
-    except Exception:
+    except NoConfigProfileError:
         return False
 
 
@@ -70,9 +76,17 @@ def switch_default_profile(profile_name):
 
 
 def create_profile(name, server, username, ignore_ssl_errors):
+    if profile_exists(name):
+        print_error(u"A profile named {} already exists.".format(name))
+        exit(1)
+
     config_accessor.create_profile(name, server, username, ignore_ssl_errors)
 
 
+def update_profile(name, server, username, ignore_ssl_errors):
+    config_accessor.update_profile(name, server, username, ignore_ssl_errors)
+
+
 def get_all_profiles():
     profiles = [Code42Profile(profile) for profile in config_accessor.get_all_profiles()]
     return profiles

tests/cmds/test_profile.py

@@ -34,6 +34,18 @@ def mock_verify(mocker):
     return mocker.patch("code42cli.cmds.profile.validate_connection")
 
 
+@pytest.fixture
+def valid_connection(mock_verify):
+    mock_verify.return_value = True
+    return mock_verify
+
+
+@pytest.fixture
+def invalid_connection(mock_verify):
+    mock_verify.return_value = False
+    return mock_verify
+
+
 def test_show_profile_outputs_profile_info(capsys, mock_cliprofile_namespace, profile):
     profile.name = "testname"
     profile.authority_url = "example.com"
@@ -58,18 +70,6 @@ def test_show_profile_when_password_set_outputs_password_note(
     assert "A password is set" not in capture.out
 
 
-def test_create_profile_if_profile_exists_exits(capsys, mock_cliprofile_namespace):
-    mock_cliprofile_namespace.profile_exists.return_value = True
-    success = True
-    try:
-        profilecmd.create_profile("foo", "bar", "baz", True)
-    except SystemExit:
-        success = True
-        capture = capsys.readouterr()
-        assert "already exists" in capture.out
-    assert success
-
-
 def test_create_profile_if_user_sets_password_is_created(
     user_agreement, mock_verify, mock_cliprofile_namespace
 ):
@@ -86,7 +86,7 @@ def test_create_profile_if_user_does_not_set_password_is_created(
     mock_cliprofile_namespace.create_profile.assert_called_once_with("foo", "bar", "baz", True)
 
 
-def test_create_profile_if_user_does_not_set_password_does_not_save_password(
+def test_create_profile_if_user_does_not_agree_does_not_save_password(
     user_disagreement, mock_verify, mock_cliprofile_namespace
 ):
     mock_cliprofile_namespace.profile_exists.return_value = False
@@ -95,9 +95,8 @@ def test_create_profile_if_user_does_not_set_password_does_not_save_password(
 
 
 def test_create_profile_if_credentials_invalid_password_not_saved(
-    user_agreement, mock_verify, mock_cliprofile_namespace
+    user_agreement, invalid_connection, mock_cliprofile_namespace
 ):
-    mock_verify.return_value = False
     mock_cliprofile_namespace.profile_exists.return_value = False
     success = False
     try:
@@ -109,14 +108,60 @@ def test_create_profile_if_credentials_invalid_password_not_saved(
 
 
 def test_create_profile_if_credentials_valid_password_saved(
-    mocker, user_agreement, mock_verify, mock_cliprofile_namespace
+    mocker, user_agreement, valid_connection, mock_cliprofile_namespace
 ):
-    mock_verify.return_value = True
     mock_cliprofile_namespace.profile_exists.return_value = False
     profilecmd.create_profile("foo", "bar", "baz", True)
     mock_cliprofile_namespace.set_password.assert_called_once_with("newpassword", mocker.ANY)
 
 
+def test_update_profile_updates_existing_profile(
+    mock_cliprofile_namespace, user_agreement, valid_connection, profile
+):
+    name = "foo"
+    profile.name = name
+    mock_cliprofile_namespace.get_profile.return_value = profile
+
+    profilecmd.update_profile(name, "bar", "baz", True)
+    mock_cliprofile_namespace.update_profile.assert_called_once_with(name, "bar", "baz", True)
+
+
+def test_update_profile_if_user_does_not_agree_does_not_save_password(
+    mock_cliprofile_namespace, user_disagreement, invalid_connection, profile
+):
+    name = "foo"
+    profile.name = name
+    mock_cliprofile_namespace.get_profile.return_value = profile
+    assert not mock_cliprofile_namespace.set_password.call_count
+
+
+def test_update_profile_if_credentials_invalid_password_not_saved(
+    user_agreement, invalid_connection, mock_cliprofile_namespace, profile
+):
+    name = "foo"
+    profile.name = name
+    mock_cliprofile_namespace.get_profile.return_value = profile
+
+    success = False
+    try:
+        profilecmd.create_profile("foo", "bar", "baz", True)
+    except SystemExit:
+        success = True
+        assert not mock_cliprofile_namespace.set_password.call_count
+    assert success
+
+
+def test_update_profile_if_user_agrees_and_valid_connection_sets_password(
+    mocker, user_agreement, valid_connection, mock_cliprofile_namespace, profile
+):
+    name = "foo"
+    profile.name = name
+    mock_cliprofile_namespace.get_profile.return_value = profile
+
+    profilecmd.update_profile(name, "bar", "baz", True)
+    mock_cliprofile_namespace.set_password.assert_called_once_with("newpassword", mocker.ANY)
+
+
 def test_prompt_for_password_reset_if_credentials_valid_password_saved(
     mocker, user_agreement, mock_verify, mock_cliprofile_namespace
 ):