From f675c57242be352682b49aaf740e3d7d0c5870fd Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 5 Feb 2016 18:48:33 -0800 Subject: [PATCH 01/11] Test no exception on empty config file --- letsencrypt/tests/cli_test.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index a5757399e..a411b7b8c 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -602,19 +602,26 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods print "Logs:" print lf.read() - - def test_renewal_verb(self): + def test_renew_verb(self): with open(test_util.vector_path('sample-renewal.conf')) as src: # put the correct path for cert.pem, chain.pem etc in the renewal conf renewal_conf = src.read().replace("MAGICDIR", test_util.vector_path()) rd = os.path.join(self.config_dir, "renewal") - os.makedirs(rd) + if not os.path.exists(rd): + os.makedirs(rd) rc = os.path.join(rd, "sample-renewal.conf") with open(rc, "w") as dest: dest.write(renewal_conf) args = ["renew", "--dry-run", "-tvv"] self._test_renewal_common(True, [], args=args, renew=True) + def test_renew_verb_empty_config(self): + renewer_configs_dir = os.path.join(self.config_dir, 'renewal') + os.makedirs(renewer_configs_dir) + with open(os.path.join(renewer_configs_dir, 'empty.conf'), 'w'): + pass # leave the file empty + self.test_renew_verb() + @mock.patch('letsencrypt.cli.zope.component.getUtility') @mock.patch('letsencrypt.cli._treat_as_renewal') @mock.patch('letsencrypt.cli._init_le_client') From ad2b6b2047abf9a154ec826e24f61a964bf01f14 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 5 Feb 2016 18:59:16 -0800 Subject: [PATCH 02/11] Test config file without renewal params --- letsencrypt/tests/cli_test.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index a411b7b8c..b2db89cd4 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -622,6 +622,20 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods pass # leave the file empty self.test_renew_verb() + def test_renew_sparse_config(self): + renewer_configs_dir = os.path.join(self.config_dir, 'renewal') + os.makedirs(renewer_configs_dir) + with open(os.path.join(renewer_configs_dir, 'test.conf'), 'w') as f: + f.write("My contents don't matter") + with mock.patch('letsencrypt.storage.RenewableCert') as mock_rc: + mock_lineage = mock.MagicMock() + mock_rc.return_value = mock_lineage + mock_lineage.configuration = ["not renewalparams"] + with mock.patch('letsencrypt.cli.obtain_cert') as mock_obtain_cert: + self._test_renewal_common(True, None, + args=['renew'], renew=False) + self.assertFalse(mock_obtain_cert.called) + @mock.patch('letsencrypt.cli.zope.component.getUtility') @mock.patch('letsencrypt.cli._treat_as_renewal') @mock.patch('letsencrypt.cli._init_le_client') From d8c0eb6d7f68113abb13643845cdc9f938d72866 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 5 Feb 2016 19:02:34 -0800 Subject: [PATCH 03/11] Test no authenticator --- letsencrypt/tests/cli_test.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index b2db89cd4..876e262ac 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -630,7 +630,12 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods with mock.patch('letsencrypt.storage.RenewableCert') as mock_rc: mock_lineage = mock.MagicMock() mock_rc.return_value = mock_lineage - mock_lineage.configuration = ["not renewalparams"] + mock_lineage.configuration = ['not renewalparams'] + with mock.patch('letsencrypt.cli.obtain_cert') as mock_obtain_cert: + self._test_renewal_common(True, None, + args=['renew'], renew=False) + self.assertFalse(mock_obtain_cert.called) + mock_lineage.configuration = {'renewalparams': ['no auth']} with mock.patch('letsencrypt.cli.obtain_cert') as mock_obtain_cert: self._test_renewal_common(True, None, args=['renew'], renew=False) From d6e207e912f20dc9e016f030f2ac3626ec839454 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 5 Feb 2016 19:11:36 -0800 Subject: [PATCH 04/11] Test renewal with bad int value in config --- letsencrypt/tests/cli_test.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 876e262ac..00588618a 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -641,6 +641,22 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods args=['renew'], renew=False) self.assertFalse(mock_obtain_cert.called) + def test_renew_with_bad_int(self): + renewer_configs_dir = os.path.join(self.config_dir, 'renewal') + os.makedirs(renewer_configs_dir) + with open(os.path.join(renewer_configs_dir, 'test.conf'), 'w') as f: + f.write("My contents don't matter") + with mock.patch('letsencrypt.storage.RenewableCert') as mock_rc: + mock_lineage = mock.MagicMock() + mock_rc.return_value = mock_lineage + mock_lineage.configuration = { + 'renewalparams': {'authenticator': None, + 'rsa_key_size': 'over 9000'}} + with mock.patch('letsencrypt.cli.obtain_cert') as mock_obtain_cert: + self._test_renewal_common(True, None, + args=['renew'], renew=False) + self.assertFalse(mock_obtain_cert.called) + @mock.patch('letsencrypt.cli.zope.component.getUtility') @mock.patch('letsencrypt.cli._treat_as_renewal') @mock.patch('letsencrypt.cli._init_le_client') From 0ba4b0c0b5dc67d07042738f6d6543f2c7ffc7c2 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 8 Feb 2016 11:42:55 -0800 Subject: [PATCH 05/11] Add bad domain renew test --- letsencrypt/tests/cli_test.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 00588618a..aebfd42a6 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -657,6 +657,22 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods args=['renew'], renew=False) self.assertFalse(mock_obtain_cert.called) + def test_renew_with_bad_domain(self): + renewer_configs_dir = os.path.join(self.config_dir, 'renewal') + os.makedirs(renewer_configs_dir) + with open(os.path.join(renewer_configs_dir, 'test.conf'), 'w') as f: + f.write("My contents don't matter") + with mock.patch('letsencrypt.storage.RenewableCert') as mock_rc: + mock_lineage = mock.MagicMock() + mock_rc.return_value = mock_lineage + mock_rc.names.return_value = ['*.example.com'] + mock_lineage.configuration = { + 'renewalparams': {'authenticator': None}} + with mock.patch('letsencrypt.cli.obtain_cert') as mock_obtain_cert: + self._test_renewal_common(True, None, + args=['renew'], renew=False) + self.assertFalse(mock_obtain_cert.called) + @mock.patch('letsencrypt.cli.zope.component.getUtility') @mock.patch('letsencrypt.cli._treat_as_renewal') @mock.patch('letsencrypt.cli._init_le_client') From 12f1ec685054fc09ee1ae59122420ab194c985c8 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 8 Feb 2016 12:11:45 -0800 Subject: [PATCH 06/11] Fix test and bad domain error handling --- letsencrypt/cli.py | 6 +++--- letsencrypt/tests/cli_test.py | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 838da4015..c335d8d5b 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -856,10 +856,10 @@ def _reconstitute(config, full_path): try: domains = [le_util.enforce_domain_sanity(x) for x in renewal_candidate.names()] - except (UnicodeError, ValueError): + except errors.ConfigurationError as error: logger.warning("Renewal configuration file %s references a cert " - "that mentions a domain name that we regarded as " - "invalid. Skipping.", full_path) + "that contains an invalid domain name. The problem " + "was: %s. Skipping.", full_path, error) return None setattr(config.namespace, "domains", domains) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index aebfd42a6..d0c2f3b91 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -650,7 +650,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods mock_lineage = mock.MagicMock() mock_rc.return_value = mock_lineage mock_lineage.configuration = { - 'renewalparams': {'authenticator': None, + 'renewalparams': {'authenticator': 'webroot', 'rsa_key_size': 'over 9000'}} with mock.patch('letsencrypt.cli.obtain_cert') as mock_obtain_cert: self._test_renewal_common(True, None, @@ -665,9 +665,9 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods with mock.patch('letsencrypt.storage.RenewableCert') as mock_rc: mock_lineage = mock.MagicMock() mock_rc.return_value = mock_lineage - mock_rc.names.return_value = ['*.example.com'] mock_lineage.configuration = { - 'renewalparams': {'authenticator': None}} + 'renewalparams': {'authenticator': 'webroot'}} + mock_lineage.names.return_value = ['*.example.com'] with mock.patch('letsencrypt.cli.obtain_cert') as mock_obtain_cert: self._test_renewal_common(True, None, args=['renew'], renew=False) From c0715d168bf32b6e4b3cec2e31ce06bf63be15cf Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 8 Feb 2016 12:29:04 -0800 Subject: [PATCH 07/11] test _restore_plugin_configs --- letsencrypt/tests/cli_test.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index d0c2f3b91..2276bad97 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -673,6 +673,24 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods args=['renew'], renew=False) self.assertFalse(mock_obtain_cert.called) + def test_renew_plugin_config_restoration(self): + renewer_configs_dir = os.path.join(self.config_dir, 'renewal') + os.makedirs(renewer_configs_dir) + with open(os.path.join(renewer_configs_dir, 'test.conf'), 'w') as f: + f.write("My contents don't matter") + with mock.patch('letsencrypt.storage.RenewableCert') as mock_rc: + mock_lineage = mock.MagicMock() + mock_rc.return_value = mock_lineage + mock_lineage.configuration = { + 'renewalparams': + {'authenticator': 'webroot', + 'webroot_path': 'None', + 'webroot_imaginary_flag': '42'}} + with mock.patch('letsencrypt.cli.obtain_cert') as mock_obtain_cert: + self._test_renewal_common(True, None, + args=['renew'], renew=False) + self.assertEqual(mock_obtain_cert.call_count, 1) + @mock.patch('letsencrypt.cli.zope.component.getUtility') @mock.patch('letsencrypt.cli._treat_as_renewal') @mock.patch('letsencrypt.cli._init_le_client') From 93ca160a1b84afed2bee67810eeb846bfe8d2af4 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 8 Feb 2016 12:34:58 -0800 Subject: [PATCH 08/11] test renew with -d/--csr --- letsencrypt/tests/cli_test.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 2276bad97..171cc0aaa 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -691,6 +691,13 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods args=['renew'], renew=False) self.assertEqual(mock_obtain_cert.call_count, 1) + def test_renew_with_bad_cli_args(self): + self.assertRaises(errors.Error, self._test_renewal_common, True, None, + args='renew -d example.com'.split(), renew=False) + self.assertRaises(errors.Error, self._test_renewal_common, True, None, + args='renew --csr {0}'.format(CSR).split(), + renew=False) + @mock.patch('letsencrypt.cli.zope.component.getUtility') @mock.patch('letsencrypt.cli._treat_as_renewal') @mock.patch('letsencrypt.cli._init_le_client') From d7be27fd847dcc6fb8e6ab829c8de9418bb45f3a Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 8 Feb 2016 12:38:18 -0800 Subject: [PATCH 09/11] Test renew catches all exceptions from reconstitute --- letsencrypt/tests/cli_test.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 171cc0aaa..a9c885f27 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -691,6 +691,19 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods args=['renew'], renew=False) self.assertEqual(mock_obtain_cert.call_count, 1) + def test_renew_reconstitute_error(self): + renewer_configs_dir = os.path.join(self.config_dir, 'renewal') + os.makedirs(renewer_configs_dir) + with open(os.path.join(renewer_configs_dir, 'test.conf'), 'w') as f: + f.write("My contents don't matter") + # pylint: disable=protected-access + with mock.patch('letsencrypt.cli._reconstitute') as mock_reconstitute: + mock_reconstitute.side_effect = [Exception] + with mock.patch('letsencrypt.cli.obtain_cert') as mock_obtain_cert: + self._test_renewal_common(True, None, + args=['renew'], renew=False) + self.assertFalse(mock_obtain_cert.called) + def test_renew_with_bad_cli_args(self): self.assertRaises(errors.Error, self._test_renewal_common, True, None, args='renew -d example.com'.split(), renew=False) From fdb9857dd8b16743286e5f28ad624a56ad242c00 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 8 Feb 2016 12:54:46 -0800 Subject: [PATCH 10/11] test obtain_cert error is caught by renew --- letsencrypt/tests/cli_test.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index a9c885f27..3647060d3 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -698,12 +698,27 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods f.write("My contents don't matter") # pylint: disable=protected-access with mock.patch('letsencrypt.cli._reconstitute') as mock_reconstitute: - mock_reconstitute.side_effect = [Exception] + mock_reconstitute.side_effect = Exception with mock.patch('letsencrypt.cli.obtain_cert') as mock_obtain_cert: self._test_renewal_common(True, None, args=['renew'], renew=False) self.assertFalse(mock_obtain_cert.called) + def test_renew_obtain_cert_error(self): + renewer_configs_dir = os.path.join(self.config_dir, 'renewal') + os.makedirs(renewer_configs_dir) + with open(os.path.join(renewer_configs_dir, 'test.conf'), 'w') as f: + f.write("My contents don't matter") + with mock.patch('letsencrypt.storage.RenewableCert') as mock_rc: + mock_lineage = mock.MagicMock() + mock_rc.return_value = mock_lineage + mock_lineage.configuration = { + 'renewalparams': {'authenticator': 'webroot'}} + with mock.patch('letsencrypt.cli.obtain_cert') as mock_obtain_cert: + mock_obtain_cert.side_effect = Exception + self._test_renewal_common(True, None, + args=['renew'], renew=False) + def test_renew_with_bad_cli_args(self): self.assertRaises(errors.Error, self._test_renewal_common, True, None, args='renew -d example.com'.split(), renew=False) From 71faa50820db415186fdd0f59a08de4bef00e350 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 8 Feb 2016 13:12:18 -0800 Subject: [PATCH 11/11] duplication-- --- letsencrypt/tests/cli_test.py | 99 +++++++++++++---------------------- 1 file changed, 35 insertions(+), 64 deletions(-) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 3647060d3..c41f45116 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -622,93 +622,64 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods pass # leave the file empty self.test_renew_verb() - def test_renew_sparse_config(self): + def _make_dummy_renewal_config(self): renewer_configs_dir = os.path.join(self.config_dir, 'renewal') os.makedirs(renewer_configs_dir) with open(os.path.join(renewer_configs_dir, 'test.conf'), 'w') as f: f.write("My contents don't matter") + + def _test_renew_common(self, renewalparams=None, + names=None, assert_oc_called=None): + self._make_dummy_renewal_config() with mock.patch('letsencrypt.storage.RenewableCert') as mock_rc: mock_lineage = mock.MagicMock() + if renewalparams is not None: + mock_lineage.configuration = {'renewalparams': renewalparams} + if names is not None: + mock_lineage.names.return_value = names mock_rc.return_value = mock_lineage - mock_lineage.configuration = ['not renewalparams'] with mock.patch('letsencrypt.cli.obtain_cert') as mock_obtain_cert: self._test_renewal_common(True, None, args=['renew'], renew=False) - self.assertFalse(mock_obtain_cert.called) - mock_lineage.configuration = {'renewalparams': ['no auth']} - with mock.patch('letsencrypt.cli.obtain_cert') as mock_obtain_cert: - self._test_renewal_common(True, None, - args=['renew'], renew=False) - self.assertFalse(mock_obtain_cert.called) + if assert_oc_called is not None: + if assert_oc_called: + self.assertTrue(mock_obtain_cert.called) + else: + self.assertFalse(mock_obtain_cert.called) + + def test_renew_no_renewalparams(self): + self._test_renew_common(assert_oc_called=False) + + def test_renew_no_authenticator(self): + self._test_renew_common(renewalparams={}, assert_oc_called=False) def test_renew_with_bad_int(self): - renewer_configs_dir = os.path.join(self.config_dir, 'renewal') - os.makedirs(renewer_configs_dir) - with open(os.path.join(renewer_configs_dir, 'test.conf'), 'w') as f: - f.write("My contents don't matter") - with mock.patch('letsencrypt.storage.RenewableCert') as mock_rc: - mock_lineage = mock.MagicMock() - mock_rc.return_value = mock_lineage - mock_lineage.configuration = { - 'renewalparams': {'authenticator': 'webroot', - 'rsa_key_size': 'over 9000'}} - with mock.patch('letsencrypt.cli.obtain_cert') as mock_obtain_cert: - self._test_renewal_common(True, None, - args=['renew'], renew=False) - self.assertFalse(mock_obtain_cert.called) + renewalparams = {'authenticator': 'webroot', + 'rsa_key_size': 'over 9000'} + self._test_renew_common(renewalparams=renewalparams, + assert_oc_called=False) def test_renew_with_bad_domain(self): - renewer_configs_dir = os.path.join(self.config_dir, 'renewal') - os.makedirs(renewer_configs_dir) - with open(os.path.join(renewer_configs_dir, 'test.conf'), 'w') as f: - f.write("My contents don't matter") - with mock.patch('letsencrypt.storage.RenewableCert') as mock_rc: - mock_lineage = mock.MagicMock() - mock_rc.return_value = mock_lineage - mock_lineage.configuration = { - 'renewalparams': {'authenticator': 'webroot'}} - mock_lineage.names.return_value = ['*.example.com'] - with mock.patch('letsencrypt.cli.obtain_cert') as mock_obtain_cert: - self._test_renewal_common(True, None, - args=['renew'], renew=False) - self.assertFalse(mock_obtain_cert.called) + renewalparams = {'authenticator': 'webroot'} + names = ['*.example.com'] + self._test_renew_common(renewalparams=renewalparams, + names=names, assert_oc_called=False) def test_renew_plugin_config_restoration(self): - renewer_configs_dir = os.path.join(self.config_dir, 'renewal') - os.makedirs(renewer_configs_dir) - with open(os.path.join(renewer_configs_dir, 'test.conf'), 'w') as f: - f.write("My contents don't matter") - with mock.patch('letsencrypt.storage.RenewableCert') as mock_rc: - mock_lineage = mock.MagicMock() - mock_rc.return_value = mock_lineage - mock_lineage.configuration = { - 'renewalparams': - {'authenticator': 'webroot', - 'webroot_path': 'None', - 'webroot_imaginary_flag': '42'}} - with mock.patch('letsencrypt.cli.obtain_cert') as mock_obtain_cert: - self._test_renewal_common(True, None, - args=['renew'], renew=False) - self.assertEqual(mock_obtain_cert.call_count, 1) + renewalparams = {'authenticator': 'webroot', + 'webroot_path': 'None', + 'webroot_imaginary_flag': '42'} + self._test_renew_common(renewalparams=renewalparams, + assert_oc_called=True) def test_renew_reconstitute_error(self): - renewer_configs_dir = os.path.join(self.config_dir, 'renewal') - os.makedirs(renewer_configs_dir) - with open(os.path.join(renewer_configs_dir, 'test.conf'), 'w') as f: - f.write("My contents don't matter") # pylint: disable=protected-access with mock.patch('letsencrypt.cli._reconstitute') as mock_reconstitute: mock_reconstitute.side_effect = Exception - with mock.patch('letsencrypt.cli.obtain_cert') as mock_obtain_cert: - self._test_renewal_common(True, None, - args=['renew'], renew=False) - self.assertFalse(mock_obtain_cert.called) + self._test_renew_common(assert_oc_called=False) def test_renew_obtain_cert_error(self): - renewer_configs_dir = os.path.join(self.config_dir, 'renewal') - os.makedirs(renewer_configs_dir) - with open(os.path.join(renewer_configs_dir, 'test.conf'), 'w') as f: - f.write("My contents don't matter") + self._make_dummy_renewal_config() with mock.patch('letsencrypt.storage.RenewableCert') as mock_rc: mock_lineage = mock.MagicMock() mock_rc.return_value = mock_lineage