From df437a0ca4785bfc5080d5272fb03412941139e5 Mon Sep 17 00:00:00 2001 From: slycoder Date: Wed, 18 Apr 2018 23:42:20 -0700 Subject: [PATCH 1/7] Fix a number of bugs relating to config file defaults * initialize should write to the default_section. * handle the case where initialize is called without a section. * handle legacy config files with default instead of defaults. * handle config files without a profile section aside from the defaults. --- onelogin_aws_cli/configuration.py | 32 +++++++++++--- .../tests/test_configurationFile.py | 44 ++++++++++++++++++- 2 files changed, 68 insertions(+), 8 deletions(-) diff --git a/onelogin_aws_cli/configuration.py b/onelogin_aws_cli/configuration.py index bdd27d22..209dea76 100644 --- a/onelogin_aws_cli/configuration.py +++ b/onelogin_aws_cli/configuration.py @@ -7,32 +7,49 @@ class ConfigurationFile(configparser.ConfigParser): """Represents a configuration ini file on disk""" + DEFAULTS = dict(save_password=False) + def __init__(self, config_file=None): super().__init__(default_section='defaults') - self['defaults'] = dict(save_password=False) + self[self.default_section] = self.DEFAULTS self.file = config_file if self.file is not None: self.load() + @property + def has_defaults(self) -> bool: + """True if the defaults section has settings beyond our defaults""" + return len(self.defaults()) > len(self.DEFAULTS) + @property def is_initialised(self) -> bool: - """True if there is at least one section""" + """True if there is at least one section or a defaults section""" return len( self.sections() - ) > 0 + ) > 0 or self.has_defaults def load(self): self.read_file(self.file) - - def initialise(self, config_name='default'): + # For backwards compatibility, we check if they have a default + # section instead of a defaults section and do a little switcheroo as + # needed. + if self.has_section('default') and not self.has_defaults: + print("It looks like you're using a the deprecated 'default' " + "section.\nConsider renaming the section to 'defaults'.") + self.default_section = 'default' + + def initialise(self, config_name=None): """ Prompt the user for configurations, and save them to the onelogin-aws-cli config file """ print("Configure Onelogin and AWS\n\n") + if not config_name: + config_name = self.default_section + config_section = self.section(config_name) config_section['base_uri'] = user_choice( @@ -68,7 +85,10 @@ def section(self, section_name: str): :param section_name: Name of the section in the config file :return: """ - if not self.has_section(section_name): + if not section_name: + section_name = self.default_section + if section_name != self.default_section and \ + not self.has_section(section_name): self.add_section(section_name) return Section(section_name, self) diff --git a/onelogin_aws_cli/tests/test_configurationFile.py b/onelogin_aws_cli/tests/test_configurationFile.py index 1da0744d..7bfbb9e1 100644 --- a/onelogin_aws_cli/tests/test_configurationFile.py +++ b/onelogin_aws_cli/tests/test_configurationFile.py @@ -60,6 +60,16 @@ def test_can_save_password_username_inherit_false(self): [profile_test]""") self.assertTrue(cfg.section("profile_test").can_save_password) + def test_can_save_password_username_defaults_false(self): + cfg = self._helper_build_config("""[defaults] +save_password = false""") + self.assertFalse(cfg.section(None).can_save_password) + + def test_can_save_password_username_defaults_true(self): + cfg = self._helper_build_config("""[defaults] +save_password = true""") + self.assertTrue(cfg.section(None).can_save_password) + def test_initialise(self): str = StringIO() cfg = ConfigurationFile(str) @@ -71,8 +81,6 @@ def test_initialise(self): self.assertEqual("""[defaults] save_password = False - -[default] base_uri = https://api.eu.onelogin.com/ client_id = mock_client_id client_secret = mock_client_secret @@ -90,3 +98,35 @@ def test_is_initialised(self): cf = self._helper_build_config("""[section] first=foo""") self.assertTrue(cf.is_initialised) + + cf = self._helper_build_config("""[defaults] +first=foo""") + self.assertTrue(cf.is_initialised) + + def test_has_defaults(self): + + content = StringIO() + cf = ConfigurationFile(content) + self.assertFalse(cf.has_defaults) + + cf = self._helper_build_config("""[defaults] +first=foo""") + self.assertTrue(cf.has_defaults) + + def test_supports_default(self): + + cf = self._helper_build_config("""[defaults] +first=foo""") + self.assertIn("first", cf.defaults()) + + cf = self._helper_build_config("""[default] +second=bar""") + self.assertIn("second", cf.defaults()) + + cf = self._helper_build_config("""[defaults] +first=foo + +[default] +second=bar""") + self.assertIn("first", cf.defaults()) + self.assertNotIn("second", cf.defaults()) From ff983484d036277ce4d4cc8f24568ca658470799 Mon Sep 17 00:00:00 2001 From: slycoder Date: Wed, 18 Apr 2018 23:45:00 -0700 Subject: [PATCH 2/7] Better message when there's no config --- onelogin_aws_cli/cli.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/onelogin_aws_cli/cli.py b/onelogin_aws_cli/cli.py index 3736616b..71d215d5 100644 --- a/onelogin_aws_cli/cli.py +++ b/onelogin_aws_cli/cli.py @@ -21,13 +21,18 @@ def _load_config(parser, config_file: ConfigurationFile, args=sys.argv[1:]): config_section = config_file.section(cli_args.config_name) - if config_section is None: + if config_section is None and cli_args.config_name: sys.exit( "Configuration '{}' not defined. " "Please run 'onelogin-aws-login -c'".format( cli_args.config_name ) ) + elif config_section is None: + sys.exit( + "No default configuration section found. " + "Please run 'onelogin-aws-login -c" + ) return config_section, cli_args From 1190f83d23301e60289fe122f54a287d3532397a Mon Sep 17 00:00:00 2001 From: slycoder Date: Wed, 18 Apr 2018 23:47:24 -0700 Subject: [PATCH 3/7] Be a bit more careful about guarding defaults --- onelogin_aws_cli/configuration.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/onelogin_aws_cli/configuration.py b/onelogin_aws_cli/configuration.py index 209dea76..becea914 100644 --- a/onelogin_aws_cli/configuration.py +++ b/onelogin_aws_cli/configuration.py @@ -12,7 +12,7 @@ class ConfigurationFile(configparser.ConfigParser): def __init__(self, config_file=None): super().__init__(default_section='defaults') - self[self.default_section] = self.DEFAULTS + self[self.default_section] = dict(self.DEFAULTS) self.file = config_file From 3fbefc174b5d40ea56143daa0a3d7aaa583846a2 Mon Sep 17 00:00:00 2001 From: slycoder Date: Thu, 19 Apr 2018 00:02:36 -0700 Subject: [PATCH 4/7] Simplify the test --- onelogin_aws_cli/configuration.py | 2 +- onelogin_aws_cli/tests/test_configurationFile.py | 7 +++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/onelogin_aws_cli/configuration.py b/onelogin_aws_cli/configuration.py index becea914..3b875953 100644 --- a/onelogin_aws_cli/configuration.py +++ b/onelogin_aws_cli/configuration.py @@ -22,7 +22,7 @@ def __init__(self, config_file=None): @property def has_defaults(self) -> bool: """True if the defaults section has settings beyond our defaults""" - return len(self.defaults()) > len(self.DEFAULTS) + return len(self[self.default_section]) > len(self.DEFAULTS) @property def is_initialised(self) -> bool: diff --git a/onelogin_aws_cli/tests/test_configurationFile.py b/onelogin_aws_cli/tests/test_configurationFile.py index 7bfbb9e1..8c6029a8 100644 --- a/onelogin_aws_cli/tests/test_configurationFile.py +++ b/onelogin_aws_cli/tests/test_configurationFile.py @@ -117,16 +117,15 @@ def test_supports_default(self): cf = self._helper_build_config("""[defaults] first=foo""") - self.assertIn("first", cf.defaults()) + self.assertEqual("defaults", cf.default_section) cf = self._helper_build_config("""[default] second=bar""") - self.assertIn("second", cf.defaults()) + self.assertEqual("default", cf.default_section) cf = self._helper_build_config("""[defaults] first=foo [default] second=bar""") - self.assertIn("first", cf.defaults()) - self.assertNotIn("second", cf.defaults()) + self.assertEqual("defaults", cf.default_section) From b8c01b6d2046ec3116d6573afcf840330dbb8498 Mon Sep 17 00:00:00 2001 From: slycoder Date: Sat, 21 Apr 2018 13:36:28 -0700 Subject: [PATCH 5/7] Revert a change --- onelogin_aws_cli/cli.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/onelogin_aws_cli/cli.py b/onelogin_aws_cli/cli.py index 2b9223e7..8874a7f1 100644 --- a/onelogin_aws_cli/cli.py +++ b/onelogin_aws_cli/cli.py @@ -23,18 +23,13 @@ def _load_config(parser, config_file: ConfigurationFile, args=sys.argv[1:]): config_section = config_file.section(cli_args.config_name) - if config_section is None and cli_args.config_name: + if config_section is None: sys.exit( "Configuration '{}' not defined. " "Please run 'onelogin-aws-login -c'".format( cli_args.config_name ) ) - elif config_section is None: - sys.exit( - "No default configuration section found. " - "Please run 'onelogin-aws-login -c" - ) return config_section, cli_args From 39db094a881ae67078d88e4897ae838b817da755 Mon Sep 17 00:00:00 2001 From: slycoder Date: Sat, 21 Apr 2018 15:12:08 -0700 Subject: [PATCH 6/7] Fix tests --- onelogin_aws_cli/configuration.py | 6 +++--- onelogin_aws_cli/tests/test_configurationFile.py | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/onelogin_aws_cli/configuration.py b/onelogin_aws_cli/configuration.py index 8fdfa357..acf68ef9 100644 --- a/onelogin_aws_cli/configuration.py +++ b/onelogin_aws_cli/configuration.py @@ -24,13 +24,13 @@ def __init__(self, config_file=None): @property def has_defaults(self) -> bool: - """True if the defaults section has settings beyond our defaults""" - return len(self[self.default_section]) > len(self.DEFAULTS) + """True if the default section is non-empty""" + return len(self.items(self.default_section)) > 0 @property def is_initialised(self) -> bool: """True if there is at least one section""" - return (len(self.sections()) > 0) + return (len(self.sections()) > 0) or self.has_defaults def load(self): self.read_file(self.file) diff --git a/onelogin_aws_cli/tests/test_configurationFile.py b/onelogin_aws_cli/tests/test_configurationFile.py index 3b585d78..455f0bce 100644 --- a/onelogin_aws_cli/tests/test_configurationFile.py +++ b/onelogin_aws_cli/tests/test_configurationFile.py @@ -63,12 +63,12 @@ def test_can_save_password_username_inherit_false(self): def test_can_save_password_username_defaults_false(self): cfg = self._helper_build_config("""[defaults] save_password = false""") - self.assertFalse(cfg.section(None).can_save_password) + self.assertFalse(cfg.section("defaults").can_save_password) def test_can_save_password_username_defaults_true(self): cfg = self._helper_build_config("""[defaults] save_password = true""") - self.assertTrue(cfg.section(None).can_save_password) + self.assertTrue(cfg.section("defaults").can_save_password) def test_initialise(self): str = StringIO() From 10826cd2208a39ca42fd0659cc50f10ba25c31ff Mon Sep 17 00:00:00 2001 From: slycoder Date: Sat, 21 Apr 2018 23:41:49 -0700 Subject: [PATCH 7/7] Remove errant lines --- onelogin_aws_cli/configuration.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/onelogin_aws_cli/configuration.py b/onelogin_aws_cli/configuration.py index acf68ef9..d7b54037 100644 --- a/onelogin_aws_cli/configuration.py +++ b/onelogin_aws_cli/configuration.py @@ -48,9 +48,6 @@ def initialise(self, config_name='defaults'): onelogin-aws-cli config file """ print("Configure Onelogin and AWS\n\n") - if not config_name: - config_name = self.default_section - config_section = self.section(config_name) if config_section is None: self.add_section(config_name)