From ad1dfc470192d18ffc0119184ab3b13bca92437b Mon Sep 17 00:00:00 2001 From: James Kasten Date: Tue, 24 Feb 2015 11:53:57 -0800 Subject: [PATCH] iteration --- letsencrypt/client/client.py | 11 +++---- letsencrypt/client/display/enhancements.py | 2 +- letsencrypt/client/display/ops.py | 17 ++++++----- letsencrypt/client/interfaces.py | 16 +++++----- letsencrypt/client/reverter.py | 17 ++++++----- letsencrypt/client/revoker.py | 14 ++++----- letsencrypt/client/tests/client_test.py | 2 +- letsencrypt/client/tests/display/ops_test.py | 30 ++++++++++++------- letsencrypt/client/tests/display/util_test.py | 29 +++++++++--------- letsencrypt/scripts/main.py | 18 ++++++++--- 10 files changed, 88 insertions(+), 68 deletions(-) diff --git a/letsencrypt/client/client.py b/letsencrypt/client/client.py index 17f72b68e..dacb3fdcc 100644 --- a/letsencrypt/client/client.py +++ b/letsencrypt/client/client.py @@ -214,7 +214,7 @@ class Client(object): :param redirect: If traffic should be forwarded from HTTP to HTTPS. :type redirect: bool or None - :raises :class:`letsencrypt.client.errors.LetsEncryptClientError`: if + :raises letsencrypt.client.errors.LetsEncryptClientError: if no installer is specified in the client. """ @@ -261,7 +261,8 @@ def validate_key_csr(privkey, csr=None): :param csr: CSR :type csr: :class:`letsencrypt.client.le_util.CSR` - :raises LetsEncryptClientError: if validation fails + :raises letsencrypt.client.errors.LetsEncryptClientError: when + validation fails """ # TODO: Handle all of these problems appropriately @@ -355,7 +356,7 @@ def determine_authenticator(all_auths): :returns: Valid Authenticator object or None - :raises :class:`letsencrypt.client.errors.LetsEncryptClientError`: If no + :raises letsencrypt.client.errors.LetsEncryptClientError: If no authenticator is available. """ @@ -380,11 +381,11 @@ def determine_authenticator(all_auths): else: raise errors.LetsEncryptClientError("No Authenticators available.") - if auth in errs: + if auth and auth in errs: logging.error("Please fix the configuration for the Authenticator. " "The following error message was received: " "%s", errs[auth]) - sys.exit(1) + return return auth diff --git a/letsencrypt/client/display/enhancements.py b/letsencrypt/client/display/enhancements.py index 25ca8e5af..d7ea3a66a 100644 --- a/letsencrypt/client/display/enhancements.py +++ b/letsencrypt/client/display/enhancements.py @@ -21,7 +21,7 @@ def ask(enhancement): :returns: True if feature is desired, False otherwise :rtype: bool - :raises :class:`letsencrypt.client.errors.LetsEncryptClientError`: If + :raises letsencrypt.client.errors.LetsEncryptClientError: If the enhancement provided is not supported. """ diff --git a/letsencrypt/client/display/ops.py b/letsencrypt/client/display/ops.py index d126b0bc6..5c644246c 100644 --- a/letsencrypt/client/display/ops.py +++ b/letsencrypt/client/display/ops.py @@ -1,6 +1,5 @@ """Contains UI methods for LE user operations.""" import os -import sys import zope.component @@ -19,7 +18,7 @@ def choose_authenticator(auths, errs): :param dict errs: Mapping IAuthenticator objects to error messages :returns: Authenticator selected - :rtype: :class:`letsencrypt.client.interfaces.IAuthenticator` + :rtype: :class:`letsencrypt.client.interfaces.IAuthenticator` or `None` """ descs = [auth.description if auth not in errs @@ -41,8 +40,7 @@ def choose_authenticator(auths, errs): util(interfaces.IDisplay).notification( msg, height=display_util.HEIGHT) else: - sys.exit(0) - + return def choose_names(installer): """Display screen to select domains to validate. @@ -50,6 +48,9 @@ def choose_names(installer): :param installer: An installer object :type installer: :class:`letsencrypt.client.interfaces.IInstaller` + :returns: List of selected names + :rtype: `list` of `str` + """ if installer is None: return _choose_names_manually() @@ -67,13 +68,13 @@ def choose_names(installer): if manual: return _choose_names_manually() else: - sys.exit(0) + return [] code, names = _filter_names(names) if code == display_util.OK and names: return names else: - sys.exit(0) + return [] def _filter_names(names): @@ -101,8 +102,8 @@ def _choose_names_manually(): if code == display_util.OK: return display_util.separate_list_input(input_) - - sys.exit(0) + # Else just return None + return [] def success_installation(domains): diff --git a/letsencrypt/client/interfaces.py b/letsencrypt/client/interfaces.py index c2dec8e62..2cb4389e1 100644 --- a/letsencrypt/client/interfaces.py +++ b/letsencrypt/client/interfaces.py @@ -18,10 +18,10 @@ class IAuthenticator(zope.interface.Interface): Finish up any additional initialization. - :raises :class:`~.errors.LetsEncryptMisconfigurationError`: when - full initialization cannot be completed. - :raises :class:`~.errors.LetsEncryptNoInstallationError`: when the - necessary programs/files cannot be located. + :raises letsencrypt.client.errors.LetsEncryptMisconfigurationError: + when full initialization cannot be completed. + :raises letsencrypt.client.errors.LetsEncryptNoInstallationError: + when the necessary programs/files cannot be located. """ @@ -135,10 +135,10 @@ class IInstaller(zope.interface.Interface): Finish up any additional initialization. - :raises :class:`~.errors.LetsEncryptMisconfigurationError`: when - full initialization cannot be completed. - :raises :class:`~.errors.LetsEncryptNoInstallationError`: when the - necessary programs/files cannot be located. + :raises letsencrypt.client.errors.LetsEncryptMisconfigurationError`: + when full initialization cannot be completed. + :raises letsencrypt.errors.LetsEncryptNoInstallationError`: + when the necessary programs/files cannot be located. """ diff --git a/letsencrypt/client/reverter.py b/letsencrypt/client/reverter.py index 75ff8b9f6..715b44f80 100644 --- a/letsencrypt/client/reverter.py +++ b/letsencrypt/client/reverter.py @@ -28,8 +28,8 @@ class Reverter(object): This function should reinstall the users original configuration files for all saves with temporary=True - :raises :class:`errors.LetsEncryptReverterError`: - Unable to revert config + :raises letsencrypt.client.errors.LetsEncryptReverterError: when + unable to revert config """ if os.path.isdir(self.config.temp_checkpoint_dir): @@ -48,7 +48,7 @@ class Reverter(object): :param int rollback: Number of checkpoints to reverse. A str num will be cast to an integer. So "2" is also acceptable. - :raises :class:`letsencrypt.client.errors.LetsEncryptReverterError`: If + :raises letsencrypt.client.errors.LetsEncryptReverterError: If there is a problem with the input or if the function is unable to correctly revert the configuration checkpoints. @@ -159,7 +159,7 @@ class Reverter(object): :param str save_notes: notes about changes made during the save :raises IOError: If unable to open cp_dir + FILEPATHS file - :raises :class:`letsencrypt.client.errors.LetsEncryptReverterError: If + :raises letsencrypt.client.errors.LetsEncryptReverterError: If unable to add checkpoint """ @@ -252,7 +252,7 @@ class Reverter(object): :param set save_files: Set of files about to be saved. - :raises :class:`letsencrypt.client.errors.LetsEncryptReverterError`: + :raises letsencrypt.client.errors.LetsEncryptReverterError: when save is attempting to overwrite a temporary file. """ @@ -288,7 +288,7 @@ class Reverter(object): a temp or permanent save. :param \*files: file paths (str) to be registered - :raises :class:`letsencrypt.client.errors.LetsEncryptReverterError`: If + :raises letsencrypt.client.errors.LetsEncryptReverterError: If call does not contain necessary parameters or if the file creation is unable to be registered. @@ -354,7 +354,7 @@ class Reverter(object): :returns: Success :rtype: bool - :raises :class:`letsencrypt.client.errors.LetsEncryptReverterError`: If + :raises letsencrypt.client.errors.LetsEncryptReverterError: If all files within file_list cannot be removed """ @@ -393,7 +393,8 @@ class Reverter(object): :param str title: Title describing checkpoint - :raises :class:`letsencrypt.client.errors.LetsEncryptReverterError` + :raises letsencrypt.client.errors.LetsEncryptReverterError: when the + checkpoint is not able to be finalized. """ # Check to make sure an "in progress" directory exists diff --git a/letsencrypt/client/revoker.py b/letsencrypt/client/revoker.py index cc075ea1f..98cf1704e 100644 --- a/letsencrypt/client/revoker.py +++ b/letsencrypt/client/revoker.py @@ -82,10 +82,8 @@ class Revoker(object): # certificate. _, b_k = self._row_to_backup(row) try: - if clean_pem == Crypto.PublicKey.RSA.importKey( - open(b_k).read()).exportKey("PEM"): - certs.append( - Cert.fromrow(row, self.config.cert_key_backup)) + test_pem = Crypto.PublicKey.RSA.importKey( + open(b_k).read()).exportKey("PEM") except (IndexError, ValueError, TypeError): # This should never happen given the assumptions of the # module. If it does, it is probably best to delete the @@ -93,6 +91,9 @@ class Revoker(object): raise errors.LetsEncryptRevokerError( "%s - backup file is corrupted.") + if clean_pem == test_pem: + certs.append( + Cert.fromrow(row, self.config.cert_key_backup)) if certs: self._safe_revoke(certs) else: @@ -216,13 +217,12 @@ class Revoker(object): if self.no_confirm or revocation.confirm_revocation(cert): try: self._acme_revoke(cert) - success_list.append(cert) - revocation.success_revocation(cert) - except errors.LetsEncryptClientError: # TODO: Improve error handling when networking is set... logging.error( "Unable to revoke cert:%s%s", os.linesep, str(cert)) + success_list.append(cert) + revocation.success_revocation(cert) finally: if success_list: self._remove_certs_keys(success_list) diff --git a/letsencrypt/client/tests/client_test.py b/letsencrypt/client/tests/client_test.py index 2a3c733fb..5b8abe91c 100644 --- a/letsencrypt/client/tests/client_test.py +++ b/letsencrypt/client/tests/client_test.py @@ -59,7 +59,7 @@ class DetermineAuthenticatorTest(unittest.TestCase): errors.LetsEncryptMisconfigurationError) mock_choose.return_value = self.mock_apache - self.assertRaises(SystemExit, self._call, self.all_auths) + self.assertRaises(self._call(self.all_auths), None) class RollbackTest(unittest.TestCase): diff --git a/letsencrypt/client/tests/display/ops_test.py b/letsencrypt/client/tests/display/ops_test.py index d0b340def..ae4bf419d 100644 --- a/letsencrypt/client/tests/display/ops_test.py +++ b/letsencrypt/client/tests/display/ops_test.py @@ -51,7 +51,7 @@ class ChooseAuthenticatorTest(unittest.TestCase): def test_no_choice(self, mock_util): mock_util().menu.return_value = (display_util.CANCEL, 0) - self.assertRaises(SystemExit, self._call, self.auths, {}) + self.assertEqual(self._call(self.auths, {}), None) class GenHttpsNamesTest(unittest.TestCase): @@ -68,14 +68,22 @@ class GenHttpsNamesTest(unittest.TestCase): self.assertEqual(self._call([]), "") def test_one(self): - dom = "example.com" - self.assertEqual(self._call([dom]), "https://%s" % dom) + doms = [ + "example.com", + "asllkjsadfljasdf.c", + ] + for dom in doms: + self.assertEqual(self._call([dom]), "https://%s" % dom) def test_two(self): - doms = ["foo.bar.org", "bar.org"] - self.assertEqual( - self._call(doms), - "https://{dom[0]} and https://{dom[1]}".format(dom=doms)) + domains_list = [ + ["foo.bar.org", "bar.org"], + ["paypal.google.facebook.live.com", "*.zombo.example.com"], + ] + for doms in domains_list: + self.assertEqual( + self._call(doms), + "https://{dom[0]} and https://{dom[1]}".format(dom=doms)) def test_three(self): doms = ["a.org", "b.org", "c.org"] @@ -112,7 +120,7 @@ class ChooseNamesTest(unittest.TestCase): @mock.patch("letsencrypt.client.display.ops.util") def test_no_installer_cancel(self, mock_util): mock_util().input.return_value = (display_util.CANCEL, []) - self.assertRaises(SystemExit, self._call, None) + self.assertEqual(self._call(None), []) @mock.patch("letsencrypt.client.display.ops.util") def test_no_names_choose(self, mock_util): @@ -130,7 +138,7 @@ class ChooseNamesTest(unittest.TestCase): self.mock_install().get_all_names.return_value = set() mock_util().yesno.return_value = False - self.assertRaises(SystemExit, self._call, self.mock_install) + self.assertEqual(self._call(self.mock_install), []) @mock.patch("letsencrypt.client.display.ops.util") def test_filter_names_valid_return(self, mock_util): @@ -146,7 +154,7 @@ class ChooseNamesTest(unittest.TestCase): self.mock_install.get_all_names.return_value = set(["example.com"]) mock_util().checklist.return_value = (display_util.OK, []) - self.assertRaises(SystemExit, self._call, self.mock_install) + self.assertEqual(self._call(self.mock_install), []) @mock.patch("letsencrypt.client.display.ops.util") def test_filter_names_cancel(self, mock_util): @@ -154,7 +162,7 @@ class ChooseNamesTest(unittest.TestCase): mock_util().checklist.return_value = ( display_util.CANCEL, ["example.com"]) - self.assertRaises(SystemExit, self._call, self.mock_install) + self.assertEqual(self._call(self.mock_install), []) class SuccessInstallationTest(unittest.TestCase): diff --git a/letsencrypt/client/tests/display/util_test.py b/letsencrypt/client/tests/display/util_test.py index e95d313b9..097401697 100644 --- a/letsencrypt/client/tests/display/util_test.py +++ b/letsencrypt/client/tests/display/util_test.py @@ -265,10 +265,10 @@ class FileOutputDisplayTest(DisplayT): def test_wrap_lines(self): # pylint: disable=protected-access - msg = ("This is just a weak test\n" - "This function is only meant to be for easy viewing\n" + msg = ("This is just a weak test{0}" + "This function is only meant to be for easy viewing{0}" "Test a really really really really really really really really " - "really really really really really long line...") + "really really really really long line...".format(os.linesep)) text = self.displayer._wrap_lines(msg) self.assertEqual(text.count(os.linesep), 3) @@ -313,20 +313,20 @@ class SeparateListInputTest(unittest.TestCase): return separate_list_input(input_) def test_commas(self): - actual = self._call("a,b,c,test") - self.assertEqual(actual, self.exp) + self.assertEqual(self._call("a,b,c,test"), self.exp) def test_spaces(self): - actual = self._call("a b c test") - self.assertEqual(actual, self.exp) + self.assertEqual(self._call("a b c test"), self.exp) def test_both(self): - actual = self._call("a, b, c, test") - self.assertEqual(actual, self.exp) + self.assertEqual(self._call("a, b, c, test"), self.exp) def test_mess(self): - actual = [self._call(" a , b c \t test")] - actual.append(self._call(",a, ,, , b c test ")) + actual = [ + self._call(" a , b c \t test"), + self._call(",a, ,, , b c test "), + self._call(",,,,, , a b,,, , c,test"), + ] for act in actual: self.assertEqual(act, self.exp) @@ -339,12 +339,11 @@ class PlaceParensTest(unittest.TestCase): return _parens_around_char(label) def test_single_letter(self): - ret = self._call("a") - self.assertEqual("(a)", ret) + self.assertEqual("(a)", self._call("a")) def test_multiple(self): - ret = self._call("Label") - self.assertEqual("(L)abel", ret) + self.assertEqual("(L)abel", self._call("Label")) + self.assertEqual("(y)es please", self._call("yes please")) if __name__ == "__main__": diff --git a/letsencrypt/scripts/main.py b/letsencrypt/scripts/main.py index 16d7177d0..892f34b7d 100755 --- a/letsencrypt/scripts/main.py +++ b/letsencrypt/scripts/main.py @@ -16,6 +16,7 @@ import letsencrypt from letsencrypt.client import configuration from letsencrypt.client import client +from letsencrypt.client import errors from letsencrypt.client import interfaces from letsencrypt.client import le_util from letsencrypt.client import log @@ -97,7 +98,7 @@ def create_parser(): return parser -def main(): # pylint: disable=too-many-branches +def main(): # pylint: disable=too-many-branches, too-many-statements """Command line argument parsing and main script execution.""" # note: arg parser internally handles --help (and exits afterwards) args = create_parser().parse_args() @@ -139,10 +140,16 @@ def main(): # pylint: disable=too-many-branches configurator.ApacheConfigurator(config), standalone.StandaloneAuthenticator(), ] - auth = client.determine_authenticator(all_auths) + try: + auth = client.determine_authenticator(all_auths) + except errors.LetsEncryptClientError: + logging.critical("No authentication mechanisms were found on your " + "system.") + sys.exit(1) + if auth is None: - logging.critical("Unable to find a way to authenticate the server.") - sys.exit(4) + logging.info("Cannot authenticate to the ACME server.") + sys.exit(0) # Use the same object if possible if interfaces.IInstaller.providedBy(auth): # pylint: disable=no-member @@ -156,6 +163,9 @@ def main(): # pylint: disable=too-many-branches else: doms = args.domains + if not doms: + sys.exit(0) + # Prepare for init of Client if args.authkey is None: authkey = client.init_key(args.rsa_key_size, config.key_dir)